Dag allen,

Ik zou heel graag feedback willen op mijn code. Tot nu toe heb ik op de procedural manier geoefend, maar wil nu overstappen op OOP.
Voordat ik dat doe, wil ik graag wat opbouwende kritiek ontvangen op wat ik tot nu toe gedaan heb :)

Komptie: linkje (verloopt over 2 weken)


NOTES:
- alleen de login/signup/settings pagina's werken.
- een db table genaamd 'clients' is nodig met de volgende velden: userID (unique,auto incr), email (unique), passw, firstname, lastname, address, city, zip, country, confirm, confirm_date, date_signup


Ben benieuwd! Be gentle ;)

Groetjes en fijn weekend, Iris
Kan je de relevante code niet in het topic plaatsen? Dat houdt het topic wat overzichtelijker.

De meeste mensen zijn ook niet zo happig om te downloaden, vrees ik.
Hoi Aar,

Dat kan ik doen, maar het zijn wel 4 verschillende paginas. Dus of het er overzichtelijker van wordt weet ik niet.
Of op Pastebin plaasten, of uploaden als *.phps (source)
Signup

 

<?php

require_once "includes/functions.php";

// store all errors in array
$errors = array();
$success = array();

// check if the form is submitted and all fields are filled in
if(isset($_POST['signup'])){

    // make sure error array is empty before we begin checking
    unset($errors);
    unset($success);

    $email = emailValidate(strip_tags($_POST['email']));
    $password = strip_tags($_POST['password']);

    // do we have any blank fields?
    if(!$email || !$password){

        $errors[] = "Please fill in all the form fields";

    }else{

        // hash the password before storing it to the db
        // @TODO move this inside function
        $hash = password_hash($password, PASSWORD_DEFAULT);

        // check if email already exists
        $user = readUser($email);

        // no, we did not find a record!
        if($user == false) {

            // add the user to the db
            newUser($email, $hash);

            // send confirmation request
            // check how to enable sending email from your localhost!
            sendConfirm($email);


            // store email in session
            $_SESSION['email'] = $email;

            // Sign up successful, go to user's settings page
            header("location: settings.php ");

        }else{

            // a record is returned, therefore the email already exists
            $errors[] = "This email address already exists";
        }
    }
}
include_once "includes/header.php";
include_once "includes/navigation.php";
?>
<div class="container-fluid">
    <div class="row">
        <div class="col-md-6">
        <h1>Sign up</h1>
            <?php
            if(!empty($errors)){
                echo "<div class=\"alert alert-danger\" role=\"alert\"><h2 class=\"h4\">Oops!</h2><ul class=\"unstyled\">";
                    foreach($errors as $error){
                        echo "<li>$error</li>";
                    }
                echo "</ul></div>";
            }
            if(!empty($success)){
                echo "<div class=\"alert alert-success\" role=\"alert\"><h2 class=\"h4\">Yipee!</h2><ul class=\"unstyled\">";
                foreach($success as $message){
                    echo "<li>$message</li>";
                }
                echo "</ul></div>";
            }?>
            <form id="signup" role="form" action="<?php echo htmlspecialchars($_SERVER['PHP_SELF']); ?>" method="post">
             <div class="form-group">
                    <label>Email</label>
                    <input type="email" name="email" class="form-control" placeholder="Email address" />
                    <label>Password</label>
                    <input type="password" name="password" class="form-control" placeholder="Your password" />
                    <hr />
                    <input type="submit" name="signup" value="Sign Up" class="btn btn-primary" />
                </div>
            </form>

            <p>Already have an account? <a href="login.php" target="_self" title="Login">Log in here</a></p>
        </div>
    </div>
</div>
<?php

include_once "includes/footer.php";

?>


Login

 
<?php

require_once "includes/functions.php";

/*
* if form is submitted, check credentials in db
* if credentials match, go to dashboard page
* if not, show error
*/

// store all errors in array
$errors = array();

if(isset($_POST['login'])){

    // make sure error array is empty before we begin checking
    unset($errors);

    $email = emailValidate(strip_tags($_POST['email']));
    $password = strip_tags($_POST['password']);

    // do we have any blank fields?
    if(!$email || !$password){

        $errors[] = "Please fill in all the form fields correctly";

    }else{

        // check if email exists and password is correct
        $user = logIn($email, $password);

        if($user != false) {

           // Yes, user is found and credentials match

            $_SESSION["email"] = $email;
            header("location: dashboard.php");

        } else {

            // credentials are incorrect
            $errors[] = "Wrong email or password, please try again";
        }

    }
}

// TODO add 'Remember me' functionality

include_once "includes/header.php";
include_once "includes/navigation.php";
?>
<div class="container-fluid">
    <div class="row">
        <div class="col-md-6">
        <h1>Login</h1>
            <?php
            if(!empty($errors)){
                echo "<div class=\"alert alert-danger\" role=\"alert\"><h2 class=\"h4\">Oops!</h2><ul class=\"unstyled\">";
                    foreach($errors as $error){
                        echo "<li>$error</li>";
                    }
                echo "</ul></div>";
            } ?>
            <form role="form" id="login" action="<?php echo htmlspecialchars($_SERVER['PHP_SELF']); ?>" method="post">
                <div class="form-group">
                    <label>Email</label>
                    <input type="email" name="email" class="form-control" placeholder="[email protected]" />
                    <label>Password</label>
                    <input type="password" name="password" class="form-control" placeholder="Your password" />
                    <hr />
                    <div class="checkbox">
                        <label>
                            <input name="remember" type="checkbox"> Remember me
                        </label>
                    </div>
                    <input type="submit" name="login" class="btn btn-primary" />
                </div>
            </form>

            <p>Need an account? <a href="signup.php" target="_self" title="Sign Up">Sign up here</a></p>
        </div>
    </div>
</div>
<?php

include_once "includes/footer.php";

?>

[size=xsmall]Toevoeging op 28/08/2015 12:25:33:[/size]

Settings

 
<?php

session_start();
if(!isset($_SESSION['email'])){
    header("location: index.php");
}
require_once "includes/functions.php";
include_once "includes/header.php";
include_once "includes/navigation.php";

// lookup user details to populate forms with current user data
$user = readUser($_SESSION['email']);

// store all messages in arrays
$errors = array();
$success = array();

// update personal details
if(isset($_POST['personal'])){

    // clear both messages arrays
    unset ($errors);
    unset ($success);

    $_POST['firstname'] = strip_tags($_POST['firstname']);
    $_POST['lastname'] = strip_tags($_POST['lastname']);
    $_POST['address'] = strip_tags($_POST['address']);
    $_POST['city'] = strip_tags($_POST['city']);
    $_POST['zip'] = strip_tags($_POST['zip']);
    $_POST['country'] = strip_tags($_POST['country']);
    $_POST['userID'] = strip_tags($_POST['userID']);

    // do we have any blank fields? No blanks allowed!
    if(!$_POST['firstname'] || !$_POST['lastname'] || !$_POST['address'] || !$_POST['city'] || !$_POST['zip'] || !$_POST['country']){

        $errors[] = "Please fill in all the form fields correctly. All fields are required.";

    }else{

        $update = updateUser($_POST);

        if ($update !== false){

            $user = $update;
            $success[] = "Your personal details are updated successfully";

        }else{

            // update failed, user wasn't found
            $errors[] = "Sorry, your details weren't updated, please try again.";

        }

    }

}
// update email
if(isset($_POST['emailupdate'])){

    // clear both messages arrays
    unset ($errors);
    unset ($success);

    $_POST['email'] = emailValidate(strip_tags($_POST['email']));
    $_POST['userID'] = strip_tags($_POST['userID']);

    // Do we have any blank fields?
    if(!$_POST['email']){

        $errors[] = "You did not enter an email address.";

    }else{

        // Let's update the email address.
        // If the update fails it means the email already exists and we throw an error

        $update = updateEmail($_POST['email'], $_POST['userID']);

        if($update !== false){

            $user = $update;
            $success[] = "Your email is updated successfully";

        }else{

            // Bummer, email already in use, show an error
            $errors[] = "This email is already in use! Please choose another.";

        }

    }

}

// update password
if(isset($_POST['password'])){

    // clear both messages arrays
    unset ($errors);
    unset ($success);

    // sanitize data input
    $old_pw = strip_tags($_POST['old_pw']);
    $new_pw = strip_tags($_POST['new_pw']);
    $check_pw = strip_tags($_POST['check_pw']);
    $userID = strip_tags($_POST['userID']);

    // no empty fields allowed
    if(!$old_pw || !$new_pw || !$check_pw){

        $errors[] = "Please fill in all the form fields correctly. All fields are required.";

    }else{

        $update = updatePw($old_pw, $new_pw, $check_pw, $userID);

        if ($update !== false){

            $success[] = "Your password is now successfully updated.";

        }else{

            $errors[] = "Sorry, your passwords didn't match. Please try again.";
        }

    }


}
?>
<div class="container-fluid">
    <div class="row">
        <div class="col-md-12">
            <h1>Settings</h1>
            <?php
            if(!empty($errors)){
                echo "<div class=\"alert alert-danger\" role=\"alert\"><h2 class=\"h4\">Oops!</h2><ul class=\"unstyled\">";
                foreach($errors as $error){
                    echo "<li>$error</li>";
                }
                echo "</ul></div>";
            }
            if(!empty($success)){
                echo "<div class=\"alert alert-success\" role=\"alert\"><h2 class=\"h4\">Yippee!</h2><ul class=\"unstyled\">";
                foreach($success as $message){
                    echo "<li>$message</li>";
                }
                echo "</ul></div>";
            }

            ?>
        </div>
        <div class="col-md-4">
            <form role="form" name="personal" action="<?php echo htmlspecialchars($_SERVER['PHP_SELF']); ?>" method="post">
                <h2>Personal details</h2>
                <div class="form-group">
                    <label>First Name</label>
                    <input type="firstname" name="firstname" class="form-control" value="<?php if (!empty($user->firstname)) echo $user->firstname; ?>" placeholder="First Name" />
                    <label>Last Name</label>
                    <input type="lastname" name="lastname" class="form-control" value="<?php if (!empty($user->lastname)) echo $user->lastname;  ?>" placeholder="Last Name" />
                    <label>Address</label>
                    <input type="address" name="address" class="form-control" value="<?php if (!empty($user->address)) echo $user->address;  ?>" placeholder="Address" />
                    <label>City</label>
                    <input type="city" name="city" class="form-control" value="<?php if (!empty($user->city)) echo $user->city;  ?>" placeholder="City" />
                    <label>Zip code</label>
                    <input type="zip" name="zip" class="form-control" value="<?php if (!empty($user->zip)) echo $user->zip;  ?>" placeholder="Zip" />
                    <label>Country</label>
                    <select type="country" name="country" class="form-control" >
                        <?php if (!empty($user->country)) echo "<option>$user->country</option>"; echo countryList();  ?>
                    </select>
                    <input type="hidden" name="userID" value="<?php echo $user->userID; ?>"/>
                </div>
                <input type="submit" name="personal" value="Update" class="btn btn-sm btn-primary" />
            </form>
            </div>
        <div class="col-md-4">
            <form role="form" name="email" action="<?php echo htmlspecialchars($_SERVER['PHP_SELF']); ?>" method="post">
                <h2>Account details</h2>
                <div class="form-group">
                    <label>Email</label>
                    <input type="email" name="email" class="form-control" value="<?php if (!empty($user->email)) echo $user->email;  ?>" placeholder="Email" />
                    <input type="hidden" name="userID" value="<?php echo $user->userID; ?>"/>
                </div>
                <input type="submit" name="emailupdate" value="Update" class="btn btn-sm btn-primary" />
            </form>
            <hr/>
            <form role="form" name="password" action="<?php echo htmlspecialchars($_SERVER['PHP_SELF']); ?>" method="post">
                <div class="form-group">
                    <label>Update your password</label>
                    <input type="password" name="old_pw" class="form-control" placeholder="Current password" />
                    <label>Choose a new password</label>
                    <input type="password" name="new_pw" class="form-control" placeholder="New password" />
                    <label>Type it again</label>
                    <input type="password" name="check_pw" class="form-control" placeholder="New password" />
                    <input type="hidden" name="userID" value="<?php echo $user->userID; ?>"/>
                </div>
                <input type="submit" name="password" value="Reset" class="btn btn-sm btn-primary" />
            </form>
        </div>
    </div>
</div>
<?php

include_once "includes/footer.php";

?>

[size=xsmall]Toevoeging op 28/08/2015 12:26:40:[/size]

Alle functies

 
<?php

// start session
session_start();


function dbConnect(){
    // connect to db. If connection fails, return false, otherwise return PDO object.

    // db settings
    $host = '';
    $dbname = '';
    $user = '';
    $pw = '';

    try{
        // connect to db and return pdo opject if connection succeeds
        $DBH = new PDO("mysql:host=$host;dbname=$dbname", $user, $pw);
        $DBH->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

        $result = $DBH;

    }catch(PDOException $e){
        $message = $e->getMessage();
        // log error in log file
        error_log($message, 3,"../logs.txt");
    }

    return $result;

}

function logIn($email, $password){

    try{

        $pdo = dbConnect();
        $stm = $pdo->prepare("SELECT email, passw
                                     FROM `clients`
                                     WHERE email = :email");
        $stm->bindParam(":email", $email);
        $stm->execute();

        // if rows are returned, the email exists and we can proceed
        if($stm->rowCount() > 0){

            // check if password matches the hash stored in the db

            $user = $stm->fetchObject();

            $hash = $user->passw;

            if (password_verify($password, $hash)) {

                // credentials are correct

                $result = $user;

            }else{

                // password is incorrect

                $result = false;
            }

        }else{

            // email doesn't exist

            $result = false;
        }

    }catch(PDOException $e){
        $message = $e->getMessage();
        // log error in log file
        error_log($message, 3,"../logs.txt");
    }

    return $result;
}

function newUser($email, $password){

    try{
        $pdo = dbConnect();
        $stm = $pdo->prepare("INSERT INTO `clients` (email, passw)
                                     VALUES (:email, :passw)");
        $stm->execute(
            array(
                ":email" => $email,
                ":passw" => $password
            )
        );

        $result = true;

    }catch(PDOException $e){
        $message = $e->getMessage();
        // log error in log file
        error_log($message, 3,"../logs.txt");
    }

    return $result;
}


function readUser($email){

    try{

        $pdo = dbConnect();
        $stm = $pdo->prepare("SELECT email,
                                            userID,
                                            firstname,
                                            lastname,
                                            address,
                                            city,
                                            zip,
                                            country
                                     FROM `clients`
                                     WHERE email = :email");

        $stm->bindParam(":email", $email);
        $stm->execute();

        // if rows are returned, the user exists and we can proceed
        if($stm->rowCount() > 0){

            $result = $stm->fetchObject();;

        }else{

            $result = false;
        }

    }catch(PDOException $e){
        $message = $e->getMessage();
        // log error in log file
        error_log($message, 3,"../logs.txt");
    }

    return $result;
}

function updateUser($user){

    try{

        if (is_array($user)) {

            // update user details, check if form is submitted

                $pdo = dbConnect();
                $stm = $pdo->prepare("UPDATE `clients` SET
                                                              firstname = :firstname,
                                                              lastname = :lastname,
                                                              address = :address,
                                                              city = :city,
                                                              zip = :zip,
                                                              country = :country
                                                              WHERE userID = :userID");
                $stm->bindParam(":firstname", $user['firstname']);
                $stm->bindParam(":lastname", $user['lastname']);
                $stm->bindParam(":address", $user['address']);
                $stm->bindParam(":city", $user['city']);
                $stm->bindParam(":zip", $user['zip']);
                $stm->bindParam(":country", $user['country']);
                $stm->bindParam(":userID", $user['userID']);
                $stm->execute();


                // if update affected any rows, the new user data will be returned
                if ($stm->rowCount() > 0) {

                    // return new user data
                    $user = readUser($_SESSION['email']);
                    $result = $user;

                } else {

                    // update failed
                    $result = false;

                }
            }

    }catch(PDOException $e) {
        $message = $e->getMessage();
        // log error in log file
        error_log($message, 3, "../logs.txt");
    }

    return $result;
}

function updateEmail($email, $userID){

    try{

        // check if the email exists in another account
        $pdo = dbConnect();
        $stm = $pdo->prepare("SELECT *
                              FROM `clients`
                              WHERE email = :email
                              AND userID <> :userID");
        $stm->bindParam(":email", $email);
        $stm->bindParam(":userID", $userID);
        $stm->execute();


        if($stm->rowCount() > 0){

            // if rows are returned, the email exists and we cannot proceed
           $result = false;

        }else{

            // email is unknown, we can update the email address!

            $stm = $pdo->prepare("UPDATE `clients` SET
                                                   email = :email
                                                   WHERE userID = :userID");
            $stm->bindParam(":email", $email);
            $stm->bindParam(":userID", $userID);
            $stm->execute();

                // if update affected any rows, the new user data will be returned
                if ($stm->rowCount() > 0) {

                    // update the session variable with the new value
                    $_SESSION['email'] = $email;

                    // return the updated user details to populate the form accordingly
                    $user = readUser($email);

                    $result = $user;

                } else {

                    $result = false;

                }
        }
    }catch (PDOException $e) {
        $message = $e->getMessage();
        // log error in log file
        error_log($message, 3, "../logs.txt");
    }

    return $result;
}

function updatePw($old, $new, $check, $userID){

    try{
        // 1) check current password with userID

        $pdo = dbConnect();
        $stm = $pdo->prepare("SELECT passw
                              FROM `clients`
                              WHERE userID = :userID");
        $stm->bindParam(":userID", $userID);
        $stm->execute();

        if($stm->rowCount() > 0){

            // we found the record, let's check the current password
            $user = $stm->fetchObject();
            $hash = $user->passw;

            if(password_verify($old, $hash)){

                // they match, we can proceed
                // 2) check if both new passwords are equal

                if($new === $check){

                    // 3) hash new password
                    $newHash = password_hash($new, PASSWORD_DEFAULT);

                    // 4) store new password in db
                    $stm = $pdo->prepare("UPDATE `clients` SET
                                                           passw = :pwHash
                                                           WHERE userID = :userID");
                    $stm->bindParam(":pwHash", $newHash);
                    $stm->bindParam(":userID", $userID);
                    $stm->execute();

                    if($stm->rowCount() > 0){

                        // we did it!

                        $result = true;

                    }else{

                        // failed!

                        $result = false;
                    }


                }else{

                    // oops, values don't match
                    $result = false;
                }

            }else{

                // wrong password!
                $result = false;
            }


        }else{

            // user not found
            $result = false;
        }


    }catch (PDOException $e) {
        $message = $e->getMessage();
        // log error in log file
        error_log($message, 3, "../logs.txt");
    }

    return $result;
}

//@TODO test on remote server
function sendConfirm($email){
    $to = $email;
    $subject = "Please confirm your email address";
    $message = "This is a test";
    $headers = "From: [email protected]" . '\r\n';
    $headers .= "Reply-to: [email protected]" . '\r\n';
    $headers .= "Content-type: text/html; charset=iso-8859-1" . '\r\n';
    $headers .= "Date:" . Date(D, M, Y) . '\r\n';

    $send = mail ($to, $subject, $message, $headers);

    return $send;
}

// email validation
function emailValidate($email){
    if(filter_var($email, FILTER_VALIDATE_EMAIL)){
        $result = $email;
    }else{
        // email is invalid, return a blank value
        $result = '';
    }

    return $result;
}

function countryList(){
    $list = array("","Afghanistan", "Albania", "ingekort");
        foreach($list as $country){
            $html .= "<option>$country</option>";
        }
    return $html;
}

[size=xsmall]Toevoeging op 28/08/2015 12:27:51:[/size]
Waar vind ik die dan? Ik zie alleen bold, underline en link etc

EDIT: Sorry, ik zie het al. Reageerde te snel en had niet op je linkje geklikt.
Ik ga het even aanpassen!
Valt weinig op aan te merken, je gebruikt netjes PDO, er zit alleen wel een lekje na je gebruik van header().
Die functie verzoekt de client naar die pagina te gaan, alleen hoeft de client hier niet naar te luisteren. Dus je zal het script moeten stoppen, met die of exit.

Johan K op 28/08/2015 12:48:28

Valt weinig op aan te merken, je gebruikt netjes PDO, er zit alleen wel een lekje na je gebruik van header().
Die functie verzoekt jouw de browser van de client naar die pagina, alleen hoeft de client hier niet naar te luisteren. Dus je zal het script moeten stoppen, met die of exit.




Bedankt Johan. Dat is leuk om te horen.

Dus de wijze waarop ik steeds connect met de db is ook goed? vai de dbConnect functie?

Ik zal de header() aanpassen en er wat over lezen om goed te begrijpen wat je zegt.
Oke, ik ben gentle en probeer het zo goed mogelijk te brengen.
Je code is nog steeds procedureel op het verbinden met PDO na.

Je code bevat een aantal fucnties die je aanroept, dit is helaas geen OOP :(

Je code is verder wel netjes maar het kan mooier.
Probeer ook geen if in een else en else in if if else else, je snapt me, te gebruiken.

Wat ook mooi is is werken met een template engine, hierdoor scheidt je je code met je view files en word het al wat gestructureerder.
Daarnaast kan je classes maken en aanroepen die bepaalde dingen moeten doen.

Je bent op een goede weg met een nog betere instelling maar hier is niet super veel OOP te bekennen.

Liefs, <== kijk netjes en lief gebracht toch
Rickert
Als je wilt controleren of een formulier verstuurd is, gebruik liever dit:


<?php 
if($_SERVER['REQUEST_METHOD']=="POST") {
// afhandeling
} else {
// niet ge-POST.
?>

Reageren