login script review - pdo

Overzicht Reageren

Sponsored by: Vacatures door Monsterboard

Top Low-Code Developer Gezocht!

Bedrijfsomschrijving Unieke Kansen, Uitstekende Arbeidsvoorwaarden & Inspirerend Team Wij zijn een toonaangevende, internationale organisatie die de toekomst van technologie vormgeeft door het creëren van innovatieve en baanbrekende oplossingen. Ons succes is gebaseerd op een hecht en gepassioneerd team van professionals die altijd streven naar het overtreffen van verwachtingen. Als jij deel wilt uitmaken van een dynamische, vooruitstrevende en inspirerende werkomgeving, dan is dit de perfecte kans voor jou! Functieomschrijving Als Low-Code Developer ben je een cruciaal onderdeel van ons team. Je werkt samen met collega's uit verschillende disciplines om geavanceerde applicaties te ontwikkelen en te optimaliseren met behulp van Low-code

Bekijk vacature »

Pagina: 1 2 volgende »

Jeroen VD

Jeroen VD

22/03/2012 18:40:25
Quote Anchor link
ik ben eigenlijk van plan dit script te posten als login script met pdo en brute-force protectie, maar ik zou vantevoren wat kritiek willen hebben over het script.

wat biedt het:
- inloggen
- gebruik gemaakt van pdo
- brute force bescherming ( maximaal 3 keer achter elkaar fout inloggen, is dat drie keer fout in 5 minuten, vijf minuten blokkade )

wat heb je nodig:
- tabel 'users' met kolommen 'user_id', 'password', 'salt'
- tabel 'loginfail' met kolommen 'user_id', 'dateAndTime'

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en">
    <head>
        <title>login</title>
        <?php
            session_start();
            if ($_SERVER['REQUEST_METHOD'] == 'POST')
            {

                if (isset($_POST['username']) && trim($_POST['username']) != '' &&
                    isset($_POST['password']) && trim($_POST['password']) != '')
                {

                    try
                    {
                        //vul hier je eigen databasegegevens in, verbinding maken met database
                        $db = new PDO('mysql:host=localhost;dbname=table', 'username', 'password');
                        //ophalen gebruikersinformatie
                        $checkUsers =
                            "SELECT
                                user_id,
                                password,
                                salt
                            FROM
                                users
                            WHERE
                                username = "
. $db->quote($_POST['username']);
                        $userResult = $db->query($checkUsers);
                        $userInf = $userResult->fetchAll();
                        if (count($userInf) > 0)
                        {

                            //ophalen inlogpogingen
                            $checkTries =
                                "SELECT
                                    TIMESTAMPDIFF(MINUTE, dateAndTime, NOW()) AS diffMinute
                                FROM
                                    loginfail
                                WHERE
                                    user_id = "
. $db->quote($userInf[0]['user_id']);
                            $triesResult = $db->query($checkTries);
                            $tries = $triesResult->fetchAll();
                            //testen hoevaak er geprobeerd is in te loggen, en hoelang geleden
                            if (count($tries) >= 3 && $tries[0]['diffMinute'] < 5)
                            {

                                $message = 'Please wait another ' . (5 - $tries[0]['diffMinute']) . ' minutes to login';
                            }

                            else
                            {
                                //wachtwoord samenstellen, testen of deze goed is
                                if (hash('sha256', ($_POST['password'] . $userInf[0]['salt'] . 'pepper')) == $userInf[0]['password'])
                                {

                                    //inlogpogingen vernietigen
                                    $deleteTries =
                                        "DELETE FROM
                                            loginfail
                                        WHERE
                                            user_id = "
. $db->quote($userInf[0]['user_id']);
                                    $deleted = $db->exec($deleteTries);
                                    $_SESSION['user'] = array('user_id' => $userInf[0]['user_id'], 'IP' => $_SERVER['REMOTE_ADDR']);
                                    //pagina waar naartoe nadat er succesvol is ingelogd
                                    header('Location: pagina.php');
                                    die;
                                }

                                else
                                {
                                    //inlogpoging toevoegen
                                    $insertTry =
                                        "INSERT INTO
                                            loginfail
                                                (user_id, dateAndTime)
                                        VALUES
                                            ("
. $db->quote($userInf[0]['user_id']) . ",
                                            NOW())"
;
                                    $inserted = $db->exec($insertTry);
                                    $message = 'wrong password';
                                }
                            }
                        }

                        else
                        {
                            $message = 'username does not exist. please fill in another';
                        }
                    }

                    catch (PDOException $e)
                    {

                        $message = $e->getMessage();
                    }
                }

                else
                {
                    $message = 'please fill in all required information';
                }
            }

        ?>
    
    </head>
    
    <body>
        <?php
            if (isset($message))
            {

                echo $message;
            }

        ?>

        <fieldset>
            <legend>log in</legend>
            <form method="post" action="login.php">
                <label for="username">username</label><br>
                <input type="text" name="username"><br>
                
                <label for="password">password</label><br>
                <input type="text" name="password"><br>
                
                <input type="submit" name="login" value="login">
            </form>
        </fieldset>
    </body>
</html>


wat vinden jullie ervan?
 
PHP hulp

PHP hulp

29/03/2024 07:51:58
 
- SanThe -

- SanThe -

22/03/2012 19:33:37
Quote Anchor link
Je begint al fout: Headers already send op regel 6.
 
Jeroen VD

Jeroen VD

22/03/2012 19:34:58
Quote Anchor link
dat snap ik niet. je moet toch altijd met een session_start() beginnen? en wanneer je dat zou doen, zou je dus geen header meer kunnen gebruiken op de hele website?
 
- Pepijn  -

- Pepijn -

22/03/2012 19:36:22
Quote Anchor link
je moet sission start helemaal boven aan zetten boven het html
 
Jeroen VD

Jeroen VD

22/03/2012 19:38:34
Quote Anchor link
dus zoiets:?
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
<?php
session_start();
?>

<html>
<head>
enzovoort
Gewijzigd op 22/03/2012 19:38:49 door Jeroen VD
 
- SanThe -

- SanThe -

22/03/2012 19:44:06
Quote Anchor link
Zet anders dit even helemaal bovenin, dan zie je het zelf:

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
<?php
ini_set('display_errors', 1); // 0 = uit, 1 = aan
error_reporting(E_ALL);

// rest
?>
 
Jeroen VD

Jeroen VD

22/03/2012 19:48:01
Quote Anchor link
geen errors, maar ik snap nog steeds niet waarom er errors zouden moeten zijn. er is toch niets naar de browser verzonden? alleen <title>login</title> staat erboven, maar dat is html, en wordt pas op de browser neergezet. dan is php toch al lang gerenderd?
 
- SanThe -

- SanThe -

22/03/2012 19:55:45
Quote Anchor link
Vóór session_start() en header() en dat soort dingen mag nog geen enkele output naar de browser zijn geweest. Dus jouw html-code die er in bovenstaand script staat, die vóór session_start() staat zal een error veroorzaken bij het bereiken van regel 6 met session_start().
 
Jeroen VD

Jeroen VD

22/03/2012 20:02:50
Quote Anchor link
Hoe zou jij dit doen dan? De <title> onder het script zetten?
 
- SanThe -

- SanThe -

22/03/2012 20:17:09
Quote Anchor link
Verder niks veranderd, alleen de logica:

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
<?php
session_start();
if ($_SERVER['REQUEST_METHOD'] == 'POST')
{

    if (isset($_POST['username']) && trim($_POST['username']) != '' &&
    isset($_POST['password']) && trim($_POST['password']) != '')
    {

        try
        {
            //vul hier je eigen databasegegevens in, verbinding maken met database
            $db = new PDO('mysql:host=localhost;dbname=table', 'username', 'password');
            //ophalen gebruikersinformatie
            $checkUsers =
                        "SELECT
                            user_id,
                            password,
                            salt
                        FROM
                            users
                        WHERE
                            username = "
. $db->quote($_POST['username']);
            $userResult = $db->query($checkUsers);
            $userInf = $userResult->fetchAll();
            if (count($userInf) > 0)
            {

                //ophalen inlogpogingen
                $checkTries =
                            "SELECT
                                TIMESTAMPDIFF(MINUTE, dateAndTime, NOW()) AS diffMinute
                            FROM
                                loginfail
                            WHERE
                                user_id = "
. $db->quote($userInf[0]['user_id']);
                $triesResult = $db->query($checkTries);
                $tries = $triesResult->fetchAll();
                //testen hoevaak er geprobeerd is in te loggen, en hoelang geleden
                if (count($tries) >= 3 && $tries[0]['diffMinute'] < 5)
                {

                    $message = 'Please wait another ' . (5 - $tries[0]['diffMinute']) . ' minutes to login';
                }

                else
                {
                    //wachtwoord samenstellen, testen of deze goed is
                    if (hash('sha256', ($_POST['password'] . $userInf[0]['salt'] . 'pepper')) == $userInf[0]['password'])
                    {

                        //inlogpogingen vernietigen
                        $deleteTries =
                                    "DELETE FROM
                                        loginfail
                                    WHERE
                                        user_id = "
. $db->quote($userInf[0]['user_id']);
                        $deleted = $db->exec($deleteTries);
                        $_SESSION['user'] = array('user_id' => $userInf[0]['user_id'], 'IP' => $_SERVER['REMOTE_ADDR']);
                        //pagina waar naartoe nadat er succesvol is ingelogd
                        header('Location: pagina.php');
                        die;
                    }

                    else
                    {
                        //inlogpoging toevoegen
                        $insertTry =
                                    "INSERT INTO
                                        loginfail
                                            (user_id, dateAndTime)
                                        VALUES
                                            ("
. $db->quote($userInf[0]['user_id']) . ",
                                            NOW())"
;
                        $inserted = $db->exec($insertTry);
                        $message = 'wrong password';
                    }
                }
            }

            else
            {
                $message = 'username does not exist. please fill in another';
            }
        }

        catch (PDOException $e)
        {

            $message = $e->getMessage();
        }
    }

    else
    {
        $message = 'please fill in all required information';
    }
}

?>
    
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en">
<head>
    <title>login</title>
</head>

<body>
<?php
if (isset($message))
{

    echo $message;
}

?>

<fieldset>
    <legend>log in</legend>
    <form method="post" action="login.php">
    <label for="username">username</label><br>
    <input type="text" name="username"><br>
    
    <label for="password">password</label><br>
    <input type="text" name="password"><br>
    
    <input type="submit" name="login" value="login">
    </form>
</fieldset>
</body>
</html>
 
Jeroen VD

Jeroen VD

22/03/2012 20:20:53
Quote Anchor link
Aah zo op die manier.... Maar wat vindt je er verder van?
 
- SanThe -

- SanThe -

22/03/2012 20:25:42
Quote Anchor link
Zo te zien wel netjes, maar ik ben geen PDO-er. ;-)
 
Erwin H

Erwin H

22/03/2012 22:29:30
Quote Anchor link
Een paar opmerkingen dan.
1) Als je toch met PDO aan de slag gaat, probeer dan ook gebruik te maken van de prepared statements. Dit is wat mij betreft de veiligste manier om gebruikersinfo in een query te gebruiken. http://php.net/manual/en/pdo.prepared-statements.php

2) Check het wachtwoord al in de query, niet er nog eens na. Wat je nu doet is username en wachtwoord ophalen en in php check je of het wachtwoord klopt. Dat kan je al doen in je query, door het naast de username op te nemen in het WHERE statement. Krijg je dan geen record terug dan weet je dat username/password combinatie niet klopte.

3) Bij de check op het aantal foute inlogpogingen, haal niet alle records op, maar bereken met een COUNT() direct in de query hoeveel pogingen er waren. Dat scheelt je mogelijk een berg records als het al heel vaak is gebeurt en is dus efficienter en sneller.

4) Sla in de failed attempts niet de userid op, maar de username en ip adres. Als er een brute force attack plaats vindt kan het goed mogelijk zijn dat men willekeurige usernames probeert, ook die niet bestaan. Als je alleen het userid opslaat dan mis je al die foute usernamen waardoor men heel veel pogingen kan doen zonder dat je het bij houdt. Daarnaast, als je ook het ip op slaat kan je ook tellen hoe vaak er vanaf een ip adres een poging is gedaan en kan je dat ip adres dus ook tijdelijk blokkeren.

Voor zover even :-)
Als je meer uitleg op bepaalde punten nodig hebt dan zie ik het wel.
Gewijzigd op 22/03/2012 22:42:02 door Erwin H
 
Nick van der heijden

nick van der heijden

23/03/2012 01:57:07
Quote Anchor link
in je querys ook gebruik maken van mysql_real_escape_string :)
 
Bart V B

Bart V B

23/03/2012 02:01:30
Quote Anchor link
Nick van der heijden op 23/03/2012 01:57:07:
in je querys ook gebruik maken van mysql_real_escape_string :)


Nope....
Hij doet:

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
$db->quote($_POST['iets']);

En dat is ook veilig. :)
 
Nick van der heijden

nick van der heijden

23/03/2012 02:10:19
Quote Anchor link
ooh oke weet ik dat ook weer ik gebruik zelf geen pdo daarom :) weer wat geleert
 
Jeroen VD

Jeroen VD

23/03/2012 16:39:14
Quote Anchor link
@erwin: ik heb drie van ja vier punten verwerkt in het onderstaande script. je hebt inderdaad gelijk qua die drie dingen. maar puntje 1 niet helemaal. een prepared statement is toch alleen nuttig wanneer je 1 query vaker gaat gebruiken, alleen met andere waarden? maar ik gebruik nu 3 (3 - eentje kon eruit) totaal verschillende queries, dus heeft het dus geen nut?

het script dan even zoals het nu is:

- tabel 'users' met kolommen 'user_id', 'password'
- tabel 'loginfail' met kolommen 'user_id', 'dateAndTime', 'loginFail_id', 'IP'
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
<?php
    ini_set('display_errors', 1); // 0 = uit, 1 = aan
    error_reporting(E_ALL);
    
    session_start();
    if ($_SERVER['REQUEST_METHOD'] == 'POST')
    {

        if (isset($_POST['username']) && trim($_POST['username']) != '' &&
            isset($_POST['password']) && trim($_POST['password']) != '')
        {

            try
            {
                //vul hier je eigen databasegegevens in, verbinding maken met database
                $db = new PDO('mysql:host=localhost;dbname=table', 'username', 'password');
                //ophalen gebruikersinformatie, testen of wachtwoord en gebruikersnaam overeenkomen
                $checkUsers =
                    "SELECT
                        user_id
                    FROM
                        users
                    WHERE
                        username = "
. $db->quote($_POST['username']) . "
                    AND
                        password = '"
. hash('sha256', $_POST['username'] . $_POST['password']) . "'";
                $userResult = $db->query($checkUsers);
                $user = $userResult->fetchAll();
                //ophalen inlogpogingen, alleen laatste vijf minuten
                $checkTries =
                    "SELECT
                        COUNT(loginFail_id) AS loginAttempts,
                        (5 - MIN(TIMESTAMPDIFF(MINUTE, dateAndTime, NOW()))) AS minuteRest,
                        dateAndTime
                    FROM
                        loginfail
                    WHERE
                        username = "
. $db->quote($_POST['username']) . "
                    HAVING
                        TIMESTAMPDIFF(MINUTE, dateAndTime, NOW()) < 5"
;
                $triesResult = $db->query($checkTries);
                $tries = $triesResult->fetchAll();
                if (count($user) == 1 && $tries[0]['loginAttempts'] < 3)
                {

                    $_SESSION['user'] = array('user_id' => $user[0]['user_id'], 'IP' => $_SERVER['REMOTE_ADDR']);
                    //pagina waar naartoe nadat er succesvol is ingelogd
                    header('Location: pagina.php');
                    die;
                }

                else
                {
                    $insertTry =
                        "INSERT INTO
                            loginfail
                                (username,
                                IP,
                                dateAndTime)
                        VALUES
                            ("
. $db->quote($_POST['username']) . ",
                            "
. $db->quote($_SERVER['REMOTE_ADDR']) . ",
                            NOW())"
;
                    $inserted = $db->exec($insertTry);
                    if(count($tries) > 0)
                    {

                        if($tries[0]['loginAttempts'] >= 3)
                        {

                            $message = 'Please wait ' . $tries[0]['minuteRest'] . ' minutes to login';
                        }

                        else
                        {
                            $message = 'invalid username/password. Please try again';
                        }
                    }

                    else
                    {
                        $message = 'invalid username/password. Please try again';
                    }
                }
            }

            catch (PDOException $e)
            {

                $message = $e->getMessage();
            }
        }

        else
        {
            $message = 'please fill in all required information';
        }
    }

?>
    
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en">
    <head>
        <title>login</title>
    </head>
    
    <body>
        <?php
            if (isset($message))
            {

                echo $message;
            }

        ?>

        <fieldset>
            <legend>log in</legend>
            <form method="post" action="login.php">
                <label for="username">username</label><br>
                <input type="text" name="username"><br>
                
                <label for="password">password</label><br>
                <input type="text" name="password"><br>
                
                <input type="submit" name="login" value="login">
            </form>
        </fieldset>
    </body>
</html>
Gewijzigd op 23/03/2012 16:56:26 door Jeroen VD
 
- Pepijn  -

- Pepijn -

23/03/2012 16:56:00
Quote Anchor link
Zet in [.code] tags aub
Gewijzigd op 23/03/2012 16:56:33 door - Pepijn -
 
Jeroen VD

Jeroen VD

23/03/2012 16:56:46
Quote Anchor link
sorry de tags fout getypt
 
Jeroen VD

Jeroen VD

24/03/2012 22:05:03
Quote Anchor link
*bump*
 
Erwin H

Erwin H

24/03/2012 22:14:19
Quote Anchor link
Jeroen vd op 23/03/2012 16:39:14:
@erwin: ik heb drie van ja vier punten verwerkt in het onderstaande script. je hebt inderdaad gelijk qua die drie dingen. maar puntje 1 niet helemaal. een prepared statement is toch alleen nuttig wanneer je 1 query vaker gaat gebruiken, alleen met andere waarden? maar ik gebruik nu 3 (3 - eentje kon eruit) totaal verschillende queries, dus heeft het dus geen nut?

Dat is zeker ook een groot voordeel, waar jij inderdaad geen gebruik van maakt. Het is echter niet het enige. Een andere reden is dat bij een prepared statement de waardes door mysql in de query worden gestopt, zonder dat je je zorgen hoeft te maken over sql injection. Als iemand sql statements erin probeert te stoppen dan gaan die gewoon als string de database in, ze zullen niet worden uitgevoerd. Je hoeft daarop dus niet expliciet te checken.

Nog een puntje over het ip adres. Als ik het goed zie dan sla je dat op als string. Het kan ook als integer wat weer ruimte scheelt, door middel van de functie INET_ATON(): http://dev.mysql.com/doc/refman/5.0/en/miscellaneous-functions.html#function_inet-aton
Die zet een IP om naar een integer waarde en met de functie INET_NTOA() zet je het weer terug.
 

Pagina: 1 2 volgende »



Overzicht Reageren

 
 

Om de gebruiksvriendelijkheid van onze website en diensten te optimaliseren maken wij gebruik van cookies. Deze cookies gebruiken wij voor functionaliteiten, analytische gegevens en marketing doeleinden. U vindt meer informatie in onze privacy statement.