-1
\$\begingroup\$

I would like to ask information to those of you that you are much more experienced than me as I am learning. I would like to create an administrative panel where the only possible option is to be logged in to the admin. Your wisest thing which is to carry out safely login?

Let me explain, I created a MySQL database table called "admin", insert a line that corresponds to the admin data, the type, (username = admin and password already inserted according to the crypt hash).

Now, by creating the login.php, I just need to verify the password (password_verify) to make sure it's safe?

Example:

<?php
session_start();
require '../includes/config.php';
$pdo->query('SELECT * FROM admin');
$row = $pdo->single();
$username = $_POST['username'];
$password = $_POST['password'];
$hash = $row['password'];
        if(isset($_POST['login'])){
            if($username == ""){
                echo "Enter username";
            }
            if($password == ""){
                echo "Enter password";
            }
            else if (password_verify($password, $hash)) {
                $_SESSION['id'] = $row['id'];
                $_SESSION['username'] = $row['username'];
                $_SESSION['level'] = $row['level'];
                $_SESSION['logged'] = time();
                header('Location: index.php');
                exit();
            } else {
                echo 'Invalid password.';
            }
        }
?>

And protected pages:

session_start();
include '../includes/config.php';
if(!isset($_SESSION['logged'])){
    header('Location: login.php');
    exit();
}

its correct and secure?

\$\endgroup\$
3
\$\begingroup\$

You have a massive bug in your application. Logging in with JUST a password will succeed, provided the password is "correct". Even if you've technically "patched" this hole because setting the location header doesn't apply if content has already been written (see documentation on header), this sort of thing shouldn't be hinging on a tiny footnote in some documentation somewhere. Not supplying a username should be a hard reject by your algorithm.

This hole, as is right now, allows users that have provided the correct password (but not the username) to obtain a session token, which they can then use by manually navigating to a protected page.

| improve this answer | |
\$\endgroup\$
2
\$\begingroup\$

Yes . But I would suggest some sort of logging each attempt on logging in .

For example if 5 failures within 5 min, wait 5 min before you can try to login again.

That would stop most scripts that would attempt to guess your password.

Another idea , you could also strict the login to one or multiple ip addresses if needed.

| improve this answer | |
\$\endgroup\$
0
\$\begingroup\$

Your database query is incorrect, because it picks single row from the admin table. If you'll have more than one admin, only one of them will be able to login.

What you should do is when your $_POST data has username and password values you should execute database query with a data user provided and check if user with that data exists.

<?php
session_start();
require '../includes/config.php';
if (isset($_POST['login']) && isset($_POST['username']) && !empty($_POST['username']) && isset($_POST['password']) && !empty($_POST['password'])) {
    $username = strip_tags(trim($_POST['username']));
    // hash your password before checking with a value in a database
    $password = hash_password(strip_tags(trim($_POST['password'])));

    // you should validate post data more to prevent sql injections.
    $pdo->query('SELECT * FROM `admin` WHERE `username` = "' . $username '" AND `password` = "' . $password . '" LIMIT 1;');
    $row = $pdo->single();

    if ($row) {
        $_SESSION['id'] = $row['id'];
        $_SESSION['username'] = $row['username'];
        $_SESSION['level'] = $row['level'];
        $_SESSION['logged'] = time();
        header('Location: index.php');
        exit();
    } else {
        // Don't tell users which field is incorrect, you don't want them to start guessing.
        echo 'Invalid username or password';
    }
} else {
    echo "Enter username and password";
}
?>
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ That code is susceptible to SQL query injection. It would be wise to utilize PDO's bound parameters... \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ May 23 '18 at 17:47

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.