4
\$\begingroup\$

I created small simple PHP Authentication API. I have a couple of scripts that I use for session, authentication and registration. Since I'm not an experienced backend and PHP developer, I wanted someone more experienced to review my scripts and tell me what I did wrong and what I can improve.

I did not use any framework; this is plain PHP.

User registration:

<?php
require_once '../dbConnect.php';

$object = json_decode(file_get_contents("php://input"), true);

if (isset($object['email']) && isset($object['password']) && isset($object['firstName']) && isset($object['lastName'])) {
    $email = $object['email'];

    $validationQuery="select * from members where email='$email'";
    $result = $mysqli->query($validationQuery) or die($mysqli->error.__LINE__);
    $member = mysqli_fetch_assoc($result);

    if($member) {
        $message = array('message' => 'Member with provided email address already exist, please use other email.');
        http_response_code(406);
        echo json_encode($message);
    } else {
        session_start();
        $firstName = $object['firstName'];
        $lastName = $object['lastName'];
        $password = password_hash($object['password'], PASSWORD_DEFAULT);

        $registrationQuery = "INSERT INTO members 
                (firstName, lastName, email, password)
                VALUES 
                ('$firstName', '$lastName', '$email', '$password')";

        if ($mysqli->query($registrationQuery) === TRUE) {
            $message = array(
                'message' => 'Registration Successful, you can use your credentials to log in.',
                'memberId' => mysqli_insert_id($mysqli));
            $_SESSION["id"] = $message['memberId'];
            echo json_encode($message);
        }
    }

    $mysqli->close();
} else {
    http_response_code(400);
}
?>

Getting authenticated member from session:

<?php
require_once '../dbConnect.php';
session_start();

$object = json_decode(file_get_contents("php://input"), true);

if (isset($object['email']) && isset($object['password'])) {

    $email = $object['email'];
    $password = $object['password'];
    $query="select * from members where email='$email'";
    $result = $mysqli->query($query) or die($mysqli->error.__LINE__);
    $member = mysqli_fetch_assoc($result);

    if($member) {
        if (password_verify($object['password'], $member['password'])) {
            $message = array('message' => 'Authentication Successful!');
            $_SESSION["id"] = $member['id'];
            echo json_encode($message);
        } else {
            $message = array('message' => 'Wrong Credentials, Authentication failed!');
            session_destroy();
            http_response_code(400);
            echo json_encode($message);
        }
    } else {
        session_destroy();
        http_response_code(406);
    }

    $mysqli->close();
} else {
    session_destroy();
    http_response_code(400);
}
?>

Getting authenticated member from PHP session cookie

<?php
require_once '../dbConnect.php';
session_start();

if (isset($_SESSION["id"])) {
    $memberId = $_SESSION["id"];
    $query="select id, firstName, lastName, email, profileImage from members where id='$memberId'";

    $result = $mysqli->query($query) or die($mysqli->error.__LINE__);
    $member = mysqli_fetch_assoc($result);

    echo $json_response = json_encode($member);

    $mysqli->close();
} else {
    http_response_code(401);
}

?>

Simple logout script:

<?php

session_start();

if (isset($_SESSION["id"])) {
    $message = array('message' => 'Successful log out!');
    session_destroy();
    echo json_encode($message);
} else {
    echo 'You are not logged in!';
    http_response_code(403);
}

?>
\$\endgroup\$
  • 1
    \$\begingroup\$ I have rolled back the last edit. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Phrancis Dec 25 '16 at 10:31
  • \$\begingroup\$ I would use pdo instead of mysqli. And why using plain PHP? Unless this is just for learning purposes, why reinventing the wheel? \$\endgroup\$ – Dan Costinel Dec 28 '16 at 22:23
3
\$\begingroup\$

Ehh, let's look at the biggest issue here: the SQL-Injection vulnerability.

$object = json_decode(file_get_contents("php://input"), true);

if (isset($object['email']) && isset($object['password']) && isset($object['firstName']) && isset($object['lastName'])) {
    $email = $object['email'];

    $validationQuery="select * from members where email='$email'";

All I have to do is provide a bad string in that JSON for email and now I can destroy your database easy.

Solution: prepared statements.

\$\endgroup\$
  • \$\begingroup\$ Can you provide quick and good example since i never used prepared statements ? \$\endgroup\$ – Super Mario's Yoshi Dec 23 '16 at 17:05
  • \$\begingroup\$ @SuperMario'sYoshi I'll see if I can whip one up here shortly. \$\endgroup\$ – 410_Gone Dec 23 '16 at 17:06
  • 1
    \$\begingroup\$ @SuperMario'sYoshi see this section of the manual for a How-To guide to prepared statements. \$\endgroup\$ – Phrancis Dec 24 '16 at 2:24
  • \$\begingroup\$ Thank you i found many examples but i am not sure what is best practice and how to implement it on my script, can anyone help me a bit with this ? \$\endgroup\$ – Super Mario's Yoshi Dec 24 '16 at 8:54
  • 1
    \$\begingroup\$ You can ask a new question if you'd like to get your new code reviewed. \$\endgroup\$ – Phrancis Dec 25 '16 at 10:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.