Aufteilung der Funktion, MVC

[vapita]

Hallo nochmal,

Ich habe jetzt den UserController etwas aufgeräumt. Ich bin mir nicht sicher, ob man es so machen sollte, zumindest gab es nach einem ersten Test keine unerwarteten Fehler. Auszugsweise dazu einmal die register-Methode:

# UserController.php

...

public function register (){
    if($this->getUser()){
        $this->redirect('302','base/index');
    }
    $userData = null;
    if($this->request->isPostRequest() && $this->request->isFormSubmitted()){
        $userData = [
            'username' => $this->request->getQuery('username'),
            'email' => $this->request->getQuery('email'),
            'password' => [
                $this->request->getQuery('password_a'),
                $this->request->getQuery('password_b'),
            ],
            'firstname' => $this->request->getQuery('firstname'),
            'lastname' => $this->request->getQuery('lastname'),
        ];
        $userRepository = $this->getRepository(User::class);
        if(is_object($user = $this->user->validate(new User(),$userRepository,$userData))) {
            $em = $this->getEntityManager();
            $em->persist($user);
            $this->flash->add(200);
            $this->redirect('302','user/login');
        }  else {
            $this->flash->add($user,'danger');
        }
    }
    $this->view->render('user/register.html.twig',[
        'flash' => $this->flash,
        'user' => $userData
    ]);
}

...

Sobald das Registrierungsformular abgesendet wurde, startet die Überprüfung, andernfalls wird einfach nur das Formular präsentiert. In der IF-Abfrage wird zuerst das Array $userData mit den POST-Daten befüllt. Die Request-Methode getQuery() macht folgendes im Hintergrund:

# Request.php

...

public function getQuery(string $FormFieldName)
{
    try {
        if ($FormFieldName !== NULL){
            $query = filter_input(INPUT_POST, $FormFieldName, FILTER_SANITIZE_STRIPPED);
            return $this->query = $query;
        } else {
            throw new Exception($query_exception);
        }
    } catch (Exception $query_exception){
        Logger::newMessage($query_exception);
        Logger::customErrorMsg($query_exception);
    }
}

...

Dann beginnt die eigentliche Prüfung in der User-Klasse von der aus auch das Passwort überprüft wird:

# User.php
class User
{

    /**
     * @var Password
     */
    public Password $password;

    /**
     * User constructor.
     */
    function __construct(){
        $this->password = new Password();
    }

    /**
     * @param $user         // User-Objekt, dass die Datenbanktabelle widerspiegelt
     * @param $repository   // Model-Repository, das die Methoden zum Abrufen enthält
     * @param array $data   // Formulardaten, die per POST übergeben worden sind
     * @return int|mixed    // Gibt entweder Fehlercode oder das User-Objekt zurück
     */
    public function validate($user, $repository, array $data)
    {
        if(0 != ($usernameLastError = $this->isString('username',$data,21031))) return $usernameLastError;
        if(0 != ($usernameLastError = $this->isUnique($repository,'username',$data,21011))) return $usernameLastError;
        if(0 != ($emailLastError = $this->isUnique($repository,'email',$data,21012))) return $emailLastError;
        if(0 != ($emailLastError = $this->isEmail('email',$data,21022))) return $emailLastError;
        if(0 != ($passwordLastError = $this->password->validate($data['password']))) return $passwordLastError;
        if(0 != ($firstnameLastError = $this->isString('firstname',$data,21034))) return $firstnameLastError;
        if(0 != ($lastnameLastError = $this->isString('lastname',$data,21035))) return $lastnameLastError;

        $user->setUsername($data['username']);
        $user->setEmail($data['email']);
        $user->setPassword($this->password->hash($data['password']));
        $user->setFirstname($data['firstname']);
        $user->setLastname($data['lastname']);
        $user->setIsActive(1);
        $user->setISBlocked(0);

        return $user;
    }

    /**
     * @param $repository
     * @param $needle
     * @param $array
     * @param int $errorCode
     * @return int|mixed
     */
    protected function isUnique($repository, $needle, $array, $errorCode = 2101){
        return ($repository->findOneBy([$needle => $array[$needle]])) ? $errorCode : 0;
    }

    /**
     * @param $needle
     * @param $array
     * @param int $errorCode
     * @return int|mixed
     */
    protected function isEmail($needle, $array, $errorCode = 2102){
        return (!filter_var($array[$needle], FILTER_VALIDATE_EMAIL)) ? $errorCode : 0;
    }

    /**
     * @param $needle
     * @param $array
     * @param int $errorCode
     * @return int|mixed
     */
    protected function isString($needle, $array, $errorCode = 2103){
        return (!is_string($array[$needle])) ? $errorCode : 0;
    }

}

Die Passwort-Klasse sieht nun so aus:

# Password.php
class Password
{

    /**
     * @param $plain_password
     * @return false|string|null
     */
    public function hash($plain_password)
    {
        $plain_password = (is_array($plain_password)) ? array_pop($plain_password) : $plain_password;
        return password_hash($plain_password, PASSWORD_DEFAULT);
    }

    /**
     * @param string $plain_password
     * @param string $correct_hash
     * @return bool
     */
    public function verify(string $plain_password, string $correct_hash)
    {
       return password_verify($plain_password, $correct_hash);
    }

    /**
     * @param array $password
     * @return int
     */
    public function validate(array $password):int
    {
        if(!$this->isString($password))     return 1601;    // kein String
        if(!$this->isEqual($password))      return 1602;    // stimmt nicht überein
        if(!$this->hasMinLength($password)) return 1603;    // ist nicht lang genug
        if(!$this->isAllowed($password))    return 1604;    // verwendet nicht erlaubte Zeichen
        if(!$this->isComplex($password))    return 1605;    // ist nicht komplex genug
        return 0; // alles in Ordnung
    }

    /**
     * @param $password
     * @return bool
     */
    private function isString($password){
        if(is_array($password)){
            foreach ($password as $item){
                $passwordIsString = (is_string($item)) ?? false;
            }
            return ($passwordIsString) ?? false;
        } else {
            return (is_string($password)) ?? false;
        }
    }

    /**
     * @param array $password
     * @return bool
     */
    private function isEqual(array $password){
        return ($password[0] == $password[1]) ?? false;
    }

    /**
     * @param $password
     * @return bool
     */
    private function hasMinLength($password){
        $password = (is_array($password)) ? array_pop($password) : $password;
        return (strlen($password) > 7) ?? false;
    }

    /**
     * @param $password
     * @return bool
     */
    private function isAllowed($password){
        // TODO: auf erlaubte Zeichen prüfen
        return true;
    }

    /**
     * @param $password
     * @return bool
     */
    private function isComplex($password){
        $password = (is_array($password)) ? array_pop($password) : $password;

        /** Das Passwort muss mindestens
         * - einen Kleinbuchstaben,
         * - einen Großbuchstaben,
         * - eine Ziffer und
         * - ein Sonderzeichen @#-_$%^&+=§!?
         * enthalten.
         * Das Passwort muss zwischen 8 und 20 Zeichen lang sein.
         */
        return (preg_match('/^(?=.*\d)(?=.*[@#\-_$%^&+=§!\?])(?=.*[a-z])(?=.*[A-Z])[0-9A-Za-z@#\-_$%^&+=§!\?]{8,20}$/',$password))? true : false;
    }

}

Hab ich da noch etwas gravierendes verkehrt gemacht oder könnte man es im groben und ganzen so stehen lassen?

Beste Grüße

vapita