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\$
2
  • 1
    \$\begingroup\$ I have rolled back the last edit. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Phrancis
    Commented Dec 25, 2016 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\$ Commented Dec 28, 2016 at 22:23

1 Answer 1

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\$
6
  • \$\begingroup\$ Can you provide quick and good example since i never used prepared statements ? \$\endgroup\$
    – Sahbaz
    Commented Dec 23, 2016 at 17:05
  • \$\begingroup\$ @SuperMario'sYoshi I'll see if I can whip one up here shortly. \$\endgroup\$ Commented Dec 23, 2016 at 17:06
  • 1
    \$\begingroup\$ @SuperMario'sYoshi see this section of the manual for a How-To guide to prepared statements. \$\endgroup\$
    – Phrancis
    Commented Dec 24, 2016 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\$
    – Sahbaz
    Commented Dec 24, 2016 at 8:54
  • 1
    \$\begingroup\$ You can ask a new question if you'd like to get your new code reviewed. \$\endgroup\$
    – Phrancis
    Commented Dec 25, 2016 at 10:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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