• Najnowsze pytania
  • Bez odpowiedzi
  • Zadaj pytanie
  • Kategorie
  • Tagi
  • Zdobyte punkty
  • Ekipa ninja
  • IRC
  • FAQ
  • Regulamin
  • Książki warte uwagi

Sprawdzenie kodu Symfony 4

VPS Starter Arubacloud
+1 głos
228 wizyt
pytanie zadane 3 lutego 2019 w PHP przez `Krzychuu Stary wyjadacz (13,940 p.)

Witam

Chciałbym żebyście sprawdzili kod na razie jest tylko rejestracja z dokumentacji symfony ale trochę przerobiłem:

<?php

namespace App\Controller;

use App\Form\UserType;
use App\Service\FormErrorsConverter;
use App\Service\Security;
use FOS\RestBundle\Controller\AbstractFOSRestController;
use FOS\RestBundle\Controller\Annotations as Rest;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Security\Core\Encoder\UserPasswordEncoderInterface;

class SecurityController extends AbstractFOSRestController
{
    /**
     * @Rest\Post("/register", name="user_register")
     *
     * @param Request $request
     * @param Security $security
     * @param FormErrorsConverter $converter
     * @param UserPasswordEncoderInterface $encoder
     *
     * @return Response
     */
    public function register(
        Request $request,
        Security $security,
        FormErrorsConverter $converter,
        UserPasswordEncoderInterface $encoder
    ): Response
    {
        $form = $this->createForm(UserType::class, null);

        $data = json_decode($request->getContent(), true);
        $form->submit($data);

        if($form->isValid()) {
            $data = $form->getData();

            $registerAction = $security->registerUser($data, $encoder);

            return $this->json([
                'success' => $registerAction
            ], Response::HTTP_OK);
        }

        return $this->json([
            'error' => "Form is not valid",
            'errors_fields' => $converter->convertErrorsFromFrom($form),
        ], Response::HTTP_BAD_REQUEST);
    }
}
<?php

namespace App\Form;

use App\Entity\User;
use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\Extension\Core\Type\EmailType;
use Symfony\Component\Form\Extension\Core\Type\PasswordType;
use Symfony\Component\Form\Extension\Core\Type\TextType;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\OptionsResolver\OptionsResolver;

class UserType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options)
    {
        $builder
            ->add('username', TextType::class)
            ->add('password', PasswordType::class)
            ->add('email', EmailType::class)
        ;
    }

    public function configureOptions(OptionsResolver $resolver)
    {
        $resolver->setDefaults([
            'data_class' => User::class,
        ]);
    }

    public function getName()
    {
        return 'user';
    }
}
<?php

namespace App\Service;

use App\Entity\User;
use App\Repository\UserRepository;
use Doctrine\ORM\EntityManagerInterface;
use Symfony\Component\Security\Core\Encoder\UserPasswordEncoderInterface;

class Security
{
    private $entityManager;
    private $userRepository;

    public function __construct(
        EntityManagerInterface $entityManager,
        UserRepository $userRepository
    )
    {
        $this->entityManager = $entityManager;
        $this->userRepository = $userRepository;
    }
    
    public function registerUser(User $user, UserPasswordEncoderInterface $encoder)
    {
        $password = $encoder->encodePassword($user, $user->getPassword());

        $user->setPassword($password);
        $this->entityManager->persist($user);
        $this->entityManager->flush();

        return $user->getId();
    }
}
<?php

namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;
use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Validator\Constraints as Assert;

/**
 * @ORM\Table(name="users")
 * @ORM\Entity(repositoryClass="App\Repository\UserRepository")
 * @UniqueEntity(
 *     fields={"username", "email"}
 * )
 *
 * @ORM\HasLifecycleCallbacks()
 */
class User implements UserInterface, \Serializable
{
    /**
     * @ORM\Id()
     * @ORM\GeneratedValue()
     * @ORM\Column(type="integer")
     */
    private $id;

    /**
     * @ORM\Column(type="string", length=50, unique=true)
     * @Assert\Length(min=3, max="50")
     * @Assert\NotBlank()
     */
    private $username;

    /**
     * @ORM\Column(type="string", length=255)
     * @Assert\NotBlank()
     * @Assert\Length(min=8)
     */
    private $password;

    /**
     * @ORM\Column(type="string", length=255, unique=true)
     * @Assert\NotBlank()
     * @Assert\Email(
     *     message = "The email '{{ value }}' is not a valid email.",
     *     checkMX = true
     * )
     */
    private $email;

    /**
     * @ORM\Column(type="boolean")
     * @Assert\NotBlank()
     */
    private $isActive;

    /**
     * @ORM\Column(type="datetime")
     * @Assert\DateTime()
     */
    private $createdAt;

    public function __construct()
    {
        $this->isActive = true;
    }

    public function getId(): ?int
    {
        return $this->id;
    }

    public function getUsername(): ?string
    {
        return $this->username;
    }

    public function setUsername(string $username): self
    {
        $this->username = $username;

        return $this;
    }

    public function getPassword(): ?string
    {
        return $this->password;
    }

    public function setPassword(string $password): self
    {
        $this->password = $password;

        return $this;
    }

    public function getEmail(): ?string
    {
        return $this->email;
    }

    public function setEmail(string $email): self
    {
        $this->email = $email;

        return $this;
    }

    public function getIsActive(): ?bool
    {
        return $this->isActive;
    }

    public function setIsActive(bool $isActive): self
    {
        $this->isActive = $isActive;

        return $this;
    }

    public function getSalt()
    {
        return null;
    }

    public function getRoles()
    {
        return array('ROLE_USER');
    }

    /**
     * @ORM\PrePersist()
     */
    public function setCreatedAt()
    {
        $this->createdAt = new \DateTime();
    }
    
    public function eraseCredentials()
    {
    }

    /** @see \Serializable::serialize() */
    public function serialize()
    {
        return serialize(array(
            $this->id,
            $this->username,
            $this->password,
        ));
    }

    /** @see \Serializable::unserialize() */
    public function unserialize($serialized)
    {
        list (
            $this->id,
            $this->username,
            $this->password,
            ) = unserialize($serialized, array('allowed_classes' => false));
    }
}

 

1 odpowiedź

+3 głosów
odpowiedź 4 lutego 2019 przez Ehlert Ekspert (212,630 p.)
wybrane 4 lutego 2019 przez `Krzychuu
 
Najlepsza
  1. json_decode w kontrolerze nie jest potrzebne. Po to mamy pole request i metodę all(). Opcjonalnie handleRequest na form.
  2. Klasa o nazwie Security. Trochę kiepsko, Wujek Bob płakałby.
  3. Dlaczego metoda z klasy Security przyjmuje passwordEncoder jako argument? Daj to do tej klasy jako zależność.
  4. Z kolei nie wiem po co w tej klasie repozytorium skoro z niego nie korzystasz. 
  5. getName jeszcze istnieje w formularzach? Na pewno nie potrzebnie. 
  6. Brakuje docblocków, ale nie wiem jakie masz konwencje. 
  7. A i strict types przydałoby się bez względu na konwencje.
  8. Duży błąd. Zauważ, że podczas całego flow rejestracji w polu usera w password raz siedzi plain password a raz hash. To nie jest dobry pomysł, w późniejszych zmianach może to prowadzić do niebezpiecznych sytuacji. Dołoż zwykłe pole na plain Password i czyść je w eraseCredentials. 
  9. Password length=255. Strzelam że korzystasz z bcrypta więc 60 znaków maksymalnie. A pro pos korzystaj z bcrypta, lub argona.
  10. Doctrine w annotations ok, ale kiedy dochodzą do tego jeszcze asercje robi się mały syf. To moje osobiste preferencje, rób jak Ci wygodniej... Żartowałem wywal asercje do ymla laugh​​​​​​
  11. Zapewne błędnie użyłeś UniqueEntity. W obecnej sytuacji email, lub username mogą się duplikować. Ich kombinacja nie może się powtórzyć.
  12. FormErrorsConverter to zupełnie zbędny byt biorąc pod uwagę, że mamy JMSSerializerBundle. Serializacja formularzy Symfony idzie mu wyśmienicie.
  13. Polecam zainstalować phpcs.
  14. Przyszłościowo dobrze byłoby trzymać role w polu. Ale to taka sugestia.
  15. Nie wszędzie masz Type hinty. 
  16. Bądź konsekwentny w typach. isActive masz domyślnie na true, asercja NotBlank(psst zmień na NotNull, bo false nie przejdzie walidacji), setter przyjmuje bool. Jakim cudem więc getter zwraca nullable.
  17. Dopisz testy devil

Ogólnie spoko. yes

komentarz 4 lutego 2019 przez `Krzychuu Stary wyjadacz (13,940 p.)
Przyznaje z nazewnictwem klas mam problem, masz rację co do UniqueEntity, teraz zauważyłem że tylko sprawdza mi username a emailu nie sprawdza i mogę dodać taki sam. Co do json_decode to trochę z tym walczyłem, dane przesyłam przez Postman w formie json i tylko z json_decode dane przeszły, gdy dodawałem bez json_decode to wywalało null. 14: tzn dodać do formularzu pole z rolami?. Mam jeszcze pytanie co do formularzy gdy mam teraz gotowy formularz User, to jakbym chciał zrobić zmianę hasła to wtedy muszę zrobić nowy formularz np ChangePasswordType?
komentarz 4 lutego 2019 przez Ehlert Ekspert (212,630 p.)

Tak, to byłoby najlepsze rozwiązanie. Od razu nakierowuje na RepeatedType.

komentarz 4 lutego 2019 przez `Krzychuu Stary wyjadacz (13,940 p.)
Mógłbyś rozwinąć punkt 14?
komentarz 4 lutego 2019 przez `Krzychuu Stary wyjadacz (13,940 p.)

@Ehlert, FormErrorsConverter zwraca tablice w której znajduje się pole i wiadomość błędu, tak wygląda kod:

{
    $errors = array();

    foreach ($form->getErrors() as $error) {
        $errors[$form->getName()][] = $error->getMessage();
    }

    foreach ($form as $child /** @var Form $child */) {
        if (!$child->isValid()) {
            foreach ($child->getErrors() as $error) {
                $errors[$child->getName()][] = $error->getMessage();
            }
        }
    }

    return $errors;
}

 

1
komentarz 4 lutego 2019 przez Ehlert Ekspert (212,630 p.)
Ty masz bardzo prostą strukturę formularza, ale zdarzają się takie bardziej skomplikowane. Nie wiem czy taki for zdałby egzamin. Dlatego instalujesz ten Serializer i wszystko śmiga.

Co do pkt 14. Dobrze jest trzymać role w polu. Może to być zwykły Varchar i masz tam wtedy jsona albo Serialize phpowy, albo doctrine ma coś takiego jak Array.
1
komentarz 4 lutego 2019 przez `Krzychuu Stary wyjadacz (13,940 p.)
teraz wszystko rozumiem, dziękuje za pomoc i poświęcony czas :)

Podobne pytania

0 głosów
1 odpowiedź 88 wizyt
pytanie zadane 30 września 2020 w PHP przez User007 Bywalec (2,400 p.)
–1 głos
1 odpowiedź 169 wizyt
pytanie zadane 10 stycznia 2020 w PHP przez niezalogowany
0 głosów
1 odpowiedź 202 wizyt

92,452 zapytań

141,262 odpowiedzi

319,085 komentarzy

61,854 pasjonatów

Motyw:

Akcja Pajacyk

Pajacyk od wielu lat dożywia dzieci. Pomóż klikając w zielony brzuszek na stronie. Dziękujemy! ♡

Oto polecana książka warta uwagi.
Pełną listę książek znajdziesz tutaj.

Akademia Sekuraka

Akademia Sekuraka 2024 zapewnia dostęp do minimum 15 szkoleń online z bezpieczeństwa IT oraz dostęp także do materiałów z edycji Sekurak Academy z roku 2023!

Przy zakupie możecie skorzystać z kodu: pasja-akademia - użyjcie go w koszyku, a uzyskacie rabat -30% na bilety w wersji "Standard"! Więcej informacji na temat akademii 2024 znajdziecie tutaj. Dziękujemy ekipie Sekuraka za taką fajną zniżkę dla wszystkich Pasjonatów!

Akademia Sekuraka

Niedawno wystartował dodruk tej świetnej, rozchwytywanej książki (około 940 stron). Mamy dla Was kod: pasja (wpiszcie go w koszyku), dzięki któremu otrzymujemy 10% zniżki - dziękujemy zaprzyjaźnionej ekipie Sekuraka za taki bonus dla Pasjonatów! Książka to pierwszy tom z serii o ITsec, który łagodnie wprowadzi w świat bezpieczeństwa IT każdą osobę - warto, polecamy!

...