[PHP] Symfony 2 Best Practices

[Antrax]

In al mijn vorige projecten in Symfony 2 gebruikte ik third-party bundles om niet constant het wiel opnieuw uit te vinden. In mijn laatste projecten (welke trouwens allemaal hobbymatig zijn) ben ik meer van het stock (om de vendors directory niet onnodig vol te stoppen). Call me a sucker, I don't care. Beetje last van perfectionisme.

In het weekend geprobeerd een nieuwe AccountController te bouwen in Symfony, wat aardig lukt, alleen vroeg ik mij af of er iemand is hier die een beetje kennis heeft van Symfony die mij kan vertellen of ik wel de juiste methodes/best-practices gebruik. Ik heb namelijk het idee dat ik ergens de plank compleet mis sla.

Alvast bedankt voor het bekijken, en sorry vanwege de grote lap code.

PHP: AccountController.php

<?php // src/AppBundle/Controller/AccountController.php

namespace AppBundle\Controller;

use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;

use AppBundle\Entity\Account;

use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\HttpFoundation\Request;

/**
 * @Route("/account")
 */
class AccountController extends Controller
{

    /**
     * @param Request $request
     * @return array
     *
     * @Route("/login", name="account_login")
     * @Method({"GET", "POST"})
     * @Security("is_granted('IS_AUTHENTICATED_ANONYMOUSLY')")
     * @Template("AppBundle:account:login.html.twig")
     */
    public function loginAction(Request $request)
    {
        $helper = $this->get('security.authentication_utils');
        $form = $this->createFormBuilder()
            ->setAction($this->generateUrl('account_login_check'))

            ->setMethod('POST')

            ->add('username', 'text', array(
                'data' => $helper->getLastUsername()
            ))

            ->add('password', 'password')

            ->add('submit', 'submit', array(
                'label' => 'Sign in'
            ))

        ->getForm();

        return array(
            'form'  => $form->createView(),
            'error' => $helper->getLastAuthenticationError()
        );
    }

    /**
     * @param Request $request
     * @throws \Exception
     *
     * @Route("/login_check", name="account_login_check")
     * @Method({"POST"})
     * @Security("is_granted('IS_AUTHENTICATED_ANONYMOUSLY')")
     */
    public function loginCheckAction(Request $request)
    {
        throw new \Exception();
    }

    /**
     * @param Request $request
     * @throws \Exception
     *
     * @Route("/logout", name="account_logout")
     * @Method({"GET"})
     * @Security("is_granted('IS_AUTHENTICATED_FULLY')")
     */
    public function logoutAction(Request $request)
    {
        throw new \Exception();
    }

    /**
     * @param Request $request
     * @return array|\Symfony\Component\HttpFoundation\RedirectResponse
     *
     * @Route("/register", name="account_register")
     * @Method({"GET", "POST"})
     * @Security("is_granted('IS_AUTHENTICATED_ANONYMOUSLY')")
     * @Template("AppBundle:account:register.html.twig")
     */
    public function registerAction(Request $request)
    {
        $form = $this->createFormBuilder($account = new Account())
            ->setAction($this->generateUrl('account_register'))

            ->setMethod('POST')

            ->add('username', 'text')

            ->add('email', 'email')

            ->add('plainPassword', 'repeated', array(
                'type'            => 'password',
                'invalid_message' => 'The password fields must match.'
            ))

            ->add('submit', 'submit', array(
                'label' => 'Sign up'
            ))

        ->getForm();

        $form->handleRequest($request);

        if ($form->isValid())
        {
            $account = $form->getData();
            $encoder = $this->container->get('security.encoder_factory')->getEncoder($account);

            $account->setConfirmationToken(md5($_SERVER['HTTP_USER_AGENT'] . time()));
            $account->setSalt(bin2hex(mcrypt_create_iv(64, MCRYPT_DEV_URANDOM)));
            $account->setPassword($encoder->encodePassword($account->getPlainPassword(),$account->getSalt()));

            $em = $this->getDoctrine()->getManager();
            $em->persist($account);
            $em->flush();

            $mailer = $this->get('mailer');

            $mailer->send($mailer->createMessage()
                ->setSubject('Activate your account')

                ->setFrom(array('from@domain' => 'From us'))

                ->setTo(array($account->getEmail() => $account->getUsername()))

                ->setBody($this->renderView('mails/register.html.twig', array(
                    'username' => $account->getUsername(),
                    'password' => $account->getPlainPassword(),
                    'code'     => $account->getConfirmationToken()
                )), 'text/html')
            );

            $this->addFlash('success', 'Your account has been created but requires activation. Please see our activation e-mail for more information.');

            return $this->redirect(
                /**
                 * ("/login", name="account_login")
                 */
                $this->generateUrl('account_login')
            );
        }

        return array(
            'form' => $form->createView()
        );
    }

    /**
     * @param Request $request
     * @param $code
     * @return \Symfony\Component\HttpFoundation\RedirectResponse
     *
     * @Route("/activate/{code}", name="account_activate")
     * @Method({"GET"})
     * @Security("is_granted('IS_AUTHENTICATED_ANONYMOUSLY')")
     */
    public function activateAction(Request $request, $code)
    {
        $account = $this->getDoctrine()->getRepository('AppBundle:Account')->loadUserByActivationCode($code);
        $em = $this->getDoctrine()->getManager();

        $account->setIsActive(true);
        $account->setConfirmationToken(null);

        $em->persist($account);
        $em->flush();

        $this->addFlash('success', 'Your account has been activated.');

        return $this->redirect(
            /**
             * ("/login", name="account_login")
             */
            $this->generateUrl('account_login')
        );
    }

    /**
     * @param Request $request
     * @return ?
     *
     * @Route("/request", name="account_password_request")
     * @Method({"GET", "POST"})
     * @Security("is_granted('IS_AUTHENTICATED_ANONYMOUSLY')")
     * @Template("AppBundle:account:forgot.html.twig")
     */
    public function forgotAction(Request $request)
    {
        //
    }

    /**
     * @param Request $request
     * @return ?
     *
     * @Route("/reset/{confirmationToken}", name="account_password_reset")
     * @Method({"GET"})
     * @Security("is_granted('IS_AUTHENTICATED_ANONYMOUSLY')")
     */
    public function newForgotAction(Request $request)
    {
        //
    }

}

[Barryvdh]

Ik werk zelf niet met Symfony 2, maar de Symfony 2 Best Practices en Demo app had je al gevonden?

http://symfony.com/blog/i...-symfony-demo-application

Today we're glad to officially introduce the Symfony Demo application. This project is a fully-functional Symfony application developed as a learning resource. The application deprecates the old AcmeDemoBundle and it can be considered the reference implementation of the Symfony Best Practices.

[Antrax]

Barryvdh schreef:
Ik werk zelf niet met Symfony 2, maar de Symfony 2 Best Practices en Demo app had je al gevonden?

Die had ik inderdaad het weekend ook al gevonden. Excuses. Dit had ik moeten vermelden in de OP.

Helaas vind ik de SecurityController wat mager en snap ik niet dat de login bijv. een aparte Controller nodig heeft (dat betekent dat ik mijn registratie en comfirm etc. allemaal in eigen Controllers moet plaatsen)?

Ik vind dat niet echt logisch.

[Geo001]

Hallo iRicardo,

Waarschijnlijk heb je wel al wat geprobeerd, maar als je het interessant vind kan ik wel een voorbeeldje plaatsen wat ik eens heb gebruikt.

Edit: Even opzoeken en beetje verzamelen tot wat makkelijk te plaatsen is

[ Voor 19% gewijzigd door Geo001 op 18-05-2015 14:53 ]

[Antrax]

Geo001 schreef op maandag 18 mei 2015 @ 14:34:
Hallo iRicardo,

Waarschijnlijk heb je wel al wat geprobeerd, maar als je het interessant vind kan ik wel een voorbeeldje plaatsen wat ik eens heb gebruikt.

Edit: Even opzoeken en beetje verzamelen tot wat makkelijk te plaatsen is

Ja leuk, DM mij anders even, dan kijken we

[Geo001]

Dit voorbeeld zet een login voor de gehele site.

In app/config/security.yml geef je aan wat je wil beveiligen:

code:

security:
  encoders:
    AppBundle\Entity\Account\User:
      id: appbundle.password.encoder

  providers:
    administrators:
      entity: { class: AppBundle\Entity\Account\User }

  firewalls:
    dev:
      pattern: ^/(_(profiler|wdt)|css|images|js)/
      security: false

    web:
      pattern: ^/
      anonymous: ~
      form_login:
        login_path:  login
        check_path:  login_check
        default_target_path:    /
      logout:
        path:   logout
        target: login
      remember_me:
        key:      "EenRandomKey"
        lifetime: 604800 # 7 days in seconds
        path:     /
        domain:   ~

    default:
        anonymous: ~



  access_control:
    - { path: ^/login, roles: IS_AUTHENTICATED_ANONYMOUSLY }
    - { path: ^/recover, roles: IS_AUTHENTICATED_ANONYMOUSLY }
    - { path: ^/, roles: [ ROLE_USER ] }

app/config/services.yml (maak de password encoder en rolehierarchy services)

code:

services:
  security.role_hierarchy:
    class:        AppBundle\Security\RoleHierarchy
    arguments:    ["%security.role_hierarchy.roles%", @doctrine.orm.default_entity_manager]
  appbundle.password.encoder:
    class: AppBundle\Security\PasswordEncoder

De password encoder: src/AppBundle/Security/PasswordEncoder.php

PHP:

<?php
namespace AppBundle\Security;
use Symfony\Component\Security\Core\Encoder\PasswordEncoderInterface;

class PasswordEncoder implements PasswordEncoderInterface{
    function encodePassword($raw, $salt)
    {
        return hash('sha512', $salt.$raw); 
    }

    function isPasswordValid($encoded, $raw, $salt)
    {
        return $encoded === $this->encodePassword($raw, $salt);
    }
}

De role hierarchy vanuit database regelen met src/AppBundle/Security/RoleHierarchy.php:

PHP:

<?php
namespace AppBundle\Security;
use Symfony\Component\Security\Core\Role\RoleHierarchy as Hierarchy;
use Doctrine\ORM\EntityManager;

class RoleHierarchy extends Hierarchy{
    private $em;

    public function __construct(array $hierarchy, EntityManager $em)
    {
        $this->em = $em;
        parent::__construct($this->buildRolesTree());
    }

    private function buildRolesTree()
    {

        $hierarchy = [];
        $roles = $this->em->createQuery('select r from AppBundle:Account\Roles r ')->execute();
        foreach ($roles as $role) {
            if ($role->getParent()) {
                if (!isset($hierarchy[$role->getParent()->getRole()])) {
                    $hierarchy[$role->getParent()->getRole()] = [];
                }
                $hierarchy[$role->getParent()->getRole()][] = $role->getRole();
            } else {
                if (!isset($hierarchy[$role->getRole()])) {
                    $hierarchy[$role->getRole()] = [];
                }
            }
        }

        return $hierarchy;
    }
}

Dan heb je nog de entities User en Roles nodig:
User:

PHP:

<?php
namespace AppBundle\Entity\Account;
use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Security\Core\User\AdvancedUserInterface;

/**
 * Class User
 * @ORM\Table(name="users")
 * @ORM\Entity(repositoryClass="AppBundle\Entity\Account\Repository\UserRepository")
 */
class User implements AdvancedUserInterface, \Serializable{
    /**
     * @ORM\Column(type="integer")
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="AUTO")
     */
    private $id;

    /**
     * @ORM\Column(type="string", length=255, unique=true)
     */
    private $username;

    /**
     * @ORM\Column(type="string", length=255)
     */
    private $password;
    private $plainPassword;

    /**
     * @ORM\Column(type="string", length=255)
     */
    private $salt;

    /**
     * @ORM\Column(type="string", length=255, unique=true)
     */
    private $email;

    /**
     * @ORM\Column(name="is_active", type="boolean")
     */
    private $isActive;

    /**
     * @ORM\ManyToMany(targetEntity="AppBundle\Entity\Account\Roles", inversedBy="users")
     */
    private $roles;
    
    public function __construct()
    {
        $this->roles = new ArrayCollection();
        $this->isActive = true;
        $this->salt = md5(uniqid(null, true));
    }

    /**
     * @inheritDoc
     */
    public function getUsername()
    {
        return $this->username;
    }

    /**
     * @inheritDoc
     */
    public function getSalt()
    {
        return $this->salt;
    }

    /**
     * @inheritDoc
     */
    public function getPassword()
    {
        return $this->password;
    }

    /**
     * @inheritDoc
     */
    public function getRoles()
    {
        return $this->roles->toArray();
    }

    /**
     * @inheritDoc
     */
    public function eraseCredentials()
    {
    }

    public function isAccountNonExpired()
    {
        return true;
    }

    public function isAccountNonLocked()
    {
        return true;
    }

    public function isCredentialsNonExpired()
    {
        return true;
    }

    public function isEnabled()
    {
        return $this->isActive;
    }

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

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

    /**
     * Get id
     *
     * @return integer 
     */
    public function getId()
    {
        return $this->id;
    }

    /**
     * Set username
     *
     * @param string $username
     * @return User
     */
    public function setUsername($username)
    {
        $this->username = $username;

        return $this;
    }

    /**
     * Set password
     *
     * @param string $password
     * @return User
     */
    public function setPassword($password)
    {
        $this->password = $password;

        return $this;
    }

    /**
     * Set salt
     *
     * @param string $salt
     * @return User
     */
    public function setSalt($salt)
    {
        $this->salt = $salt;

        return $this;
    }

    /**
     * Set email
     *
     * @param string $email
     * @return User
     */
    public function setEmail($email)
    {
        $this->email = $email;

        return $this;
    }

    /**
     * Get email
     *
     * @return string
     */
    public function getEmail()
    {
        return $this->email;
    }

    /**
     * Set plainPassword
     *
     * @param string $plainPassword
     * @return User
     */
    public function setPlainPassword($plainPassword)
    {
        $this->plainPassword = $plainPassword;

        return $this;
    }

    /**
     * Get plainPassword
     *
     * @return string
     */
    public function getPlainPassword()
    {
        return $this->plainPassword;
    }

    /**
     * Set isActive
     *
     * @param boolean $isActive
     * @return User
     */
    public function setIsActive($isActive)
    {
        $this->isActive = $isActive;

        return $this;
    }

    /**
     * Get isActive
     *
     * @return boolean 
     */
    public function getIsActive()
    {
        return $this->isActive;
    }


    public function addRole(\AppBundle\Entity\Account\Roles $roles)
    {
        $this->roles[] = $roles;

        return $this;
    }


    public function removeRole(\AppBundle\Entity\Account\Roles $roles)
    {
        $this->roles->removeElement($roles);
    }
}

Roles:

PHP:

<?php
namespace AppBundle\Entity\Account;

use Symfony\Component\Security\Core\Role\RoleInterface;
use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\ORM\Mapping as ORM;

/**
 * Class Role
 * @package AppBundle\Entity\Account
 * @ORM\Table(name="role")
 * @ORM\Entity()
 */
class Roles implements RoleInterface{
    /**
     * @ORM\Column(name="id", type="integer")
     * @ORM\Id()
     * @ORM\GeneratedValue(strategy="AUTO")
     */
    private $id;

    /**
     * @ORM\Column(name="name", type="string", length=30)
     */
    private $name;

    /**
     * @ORM\Column(name="role", type="string", length=35, unique=true)
     */
    private $role;

    /**
     * @ORM\ManyToMany(targetEntity="AppBundle\Entity\Account\User", mappedBy="roles")
     */
    private $users;

    /**
     * @ORM\ManyToOne(targetEntity="AppBundle\Entity\Account\Roles", inversedBy="children", cascade={"persist"})
     * @ORM\JoinColumn(name="parent_id", referencedColumnName="id", onDelete="SET NULL")
     **/
    private $parent;

    /**
     * @var \Doctrine\Common\Collections\Collection $children
     * @ORM\OneToMany(targetEntity="AppBundle\Entity\Account\Roles", mappedBy="parent")
     */
    private $children;

    public function __construct()
    {
        $this->users = new ArrayCollection();
    }

    /**
     * @see RoleInterface
     */
    public function getRole()
    {
        return $this->role;
    }

    function __toString()
    {
        return $this->name;
    }

    /**
     * Get id
     *
     * @return integer 
     */
    public function getId()
    {
        return $this->id;
    }

    /**
     * Set name
     *
     * @param string $name
     * @return Roles
     */
    public function setName($name)
    {
        $this->name = $name;

        return $this;
    }

    /**
     * Get name
     *
     * @return string 
     */
    public function getName()
    {
        return $this->name;
    }

    /**
     * Set role
     *
     * @param string $role
     * @return Roles
     */
    public function setRole($role)
    {
        $this->role = $role;

        return $this;
    }

    /**
     * Add users
     *
     * @param \AppBundle\Entity\Account\User $users
     * @return Roles
     */
    public function addUser(\AppBundle\Entity\Account\User $users)
    {
        $this->users[] = $users;

        return $this;
    }

    /**
     * Remove users
     *
     * @param \AppBundle\Entity\Account\User $users
     */
    public function removeUser(\AppBundle\Entity\Account\User $users)
    {
        $this->users->removeElement($users);
    }

    /**
     * Get users
     *
     * @return \Doctrine\Common\Collections\Collection 
     */
    public function getUsers()
    {
        return $this->users;
    }

    /**
     * Set parent
     *
     * @param \AppBundle\Entity\Account\Roles $parent
     * @return Roles
     */
    public function setParent(\AppBundle\Entity\Account\Roles $parent = null)
    {
        $this->parent = $parent;

        return $this;
    }

    /**
     * Get parent
     *
     * @return \AppBundle\Entity\Account\Roles 
     */
    public function getParent()
    {
        return $this->parent;
    }

    /**
     * Add children
     *
     * @param \AppBundle\Entity\Account\Roles $children
     * @return Roles
     */
    public function addChild(\AppBundle\Entity\Account\Roles $children)
    {
        $this->children[] = $children;

        return $this;
    }

    /**
     * Remove children
     *
     * @param \AppBundle\Entity\Account\Roles $children
     * @param \AppBundle\Entity\Account\Roles $children
     */
    public function removeChild(\AppBundle\Entity\Account\Roles $children)
    {
        $this->children->removeElement($children);
    }

    /**
     * Get children
     *
     * @return \Doctrine\Common\Collections\Collection 
     */
    public function getChildren()
    {
        return $this->children;
    }
}

UserRepository

PHP:

<?php

namespace AppBundle\Entity\Account\Repository;

use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Core\User\UserProviderInterface;
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
use Doctrine\ORM\EntityRepository;
use Doctrine\ORM\NoResultException;

/**
 * Class UserRepository
 * @package AppBundle\Entity\Account\Repository
 */
class UserRepository extends EntityRepository implements UserProviderInterface{
    public function loadUserByUsername($username)
    {
        $q = $this
            ->createQueryBuilder('u')
            ->select('u, r')
            ->leftJoin('u.roles', 'r')
            ->where('u.username = :username OR u.email = :email')
            ->setParameter('username', $username)
            ->setParameter('email', $username)
            ->getQuery();

        try {
            // The Query::getSingleResult() method throws an exception
            // if there is no record matching the criteria.
            $user = $q->getSingleResult();
        } catch (NoResultException $e) {
            $message = sprintf(
                'Unable to find an active account AppBundle:Account\User object identified by "%s".',
                $username
            );
            throw new UsernameNotFoundException($message, 0, $e);
        }

        return $user;
    }

    public function refreshUser(UserInterface $user)
    {
        $class = get_class($user);
        if (!$this->supportsClass($class)) {
            throw new UnsupportedUserException(
                sprintf(
                    'Instances of "%s" are not supported.',
                    $class
                )
            );
        }

        return $this->find($user->getId());
    }

    public function supportsClass($class)
    {
        return $this->getEntityName() === $class
        || is_subclass_of($class, $this->getEntityName());
    }
}

dan in de controller:

PHP:

    /**
     * @Route("/login", name="login")
     * @Template()
     */
    public function LoginAction()
    {
        return array();
    }
    
    /**
     * @Route("/login_check", name="login_check")
     */
    public function loginCheckAction() {}

    /**
     * @Route("/logout", name="logout")
     */
    public function logoutAction() {}

en als laatste de twig:

code:

<form action="{{ path('login_check') }}" method="post">
    <input type="text" autocomplete="off" placeholder="Username" name="_username"/>
    <input type="password" autocomplete="off" placeholder="Password" name="_password"/>
    <input type="checkbox" name="_remember_me" value="1"/>
    <button type="submit">Login</button>
</form>

[Antrax]

Bedank voor je code, ik zit er niet ver naast zo te zien. Ik zou wel mijn firewall wat kunnen verbeteren. Als je wilt, kan ik de rest van de bundle sharen zodat je zelf ook een goed punt hebt om op verder te bouwen

[XyritZz]

Als je serieus met Symfony aan de slag wil gaan; kijk dan ook even naar de standaard bundles die al beschikbaar zijn. Zeker voor useraccounts etc. is er een erg goede bundle vrij beschikbaar; de FOS Userbundle. Zonde van je tijd om het wiel opnieuw uit te vinden toch? .

Inhoudelijk over je eerdere posts:

Helaas vind ik de SecurityController wat mager en snap ik niet dat de login bijv. een aparte Controller nodig heeft (dat betekent dat ik mijn registratie en comfirm etc. allemaal in eigen Controllers moet plaatsen)?

Dat hoeft niet, maar is een kwestie van smaak en het opdelen van verantwoordelijkheden. Registratie/confirm kan gewoon in de UserController, maar login zou ik daar persoonlijk niet bij plaatsen.

Het "mager" zijn van een controller is niet echt een goed argument, controllers zo compact mogelijk houden kan later een beter te beheren applicatie opleveren. Eén actie in een controller is voor mij geen zeldzaamheid, vaak zie je dan toch dat er op een later moment nog wel eens een extra actie bij komt.

Verder zou ik zodra een formulier zichtbare velden bevat het verplaatsen naar een FormType. In je controller het hele formulier opbouwen kan wel als je bijv. een Delete formulier wil maken, dat is vaak niet meer dan:

PHP:

$this->createFormBuilder()
       ->setAction($this->generateUrl('delete_user', ['id' => $id]))
       ->setMethod('DELETE')
       ->add('submit', 'submit', ['label' => 'Delete'])
       ->getForm();

Complexere formulieren gewoon meteen in een FormType gooien, Nu zijn ze nog relatief klein, maar zodra je meerdere attributes per veld gaat definieren groeit die code best vlot en dan is het fijn om het meteen in een aparte class te zetten.

Hoe je FormTypes maakt kun je in de documentatie wel terug vinden; volges mij kan: app/console generate:doctrine:form je ook aardig op weg helpen.

In je controller heb je dan ipv het hele formulier slechts een paar regels code nodig:

PHP:

$form = $this->createForm(
            new UserType(),
            $entity,
            [
                'action' => $this->generateUrl('create_user'),
                'method' => 'POST'
            ]
        );

Dit kun je ook heel makkelijk hergebruiken, met een andere URL (edit_user) en andere Method (PUT) kun je dit ook meteen als je edit formulier gebruiken.

Zo kun je nog verder gaan met het verplaatsen van code uit je controller naar aparte classes, zo is het ook altijd mijn streven om de hele entity manager niet te gebruiken in een controller. Laat maar weten als je er interesse in hebt om te weten hoe ik dat doe, dan wil ik daar best wat uitleg/voorbeelden van geven.