Keskustelu: Nettisivujen teko: MySQL-kirjautuminen

Terve!

Tein tossa jokunen kuukaisi sitten sellaisen kirjautumissysteemin, joka haki tunnusparit tekstitiedostosta, ja nyt sitten päätin toteuttaa saman MySQL -tietokannalla.

<?php

//Muodostetaan yhteys ja valitaan tietokanta
$yhteys = mysql_connect("localhost","root","********");
mysql_select_db("customdata",$yhteys);

//Tuodaan tunnus ja salasana -tiedot turvallisesti sovellukseen ja hashataan salasana
$hashpass = md5($_POST['pass']);
$user = $_POST['user'];

//MySQL -kysely: Etsii oikeita pareja tietokannasta
$kysely = mysql_query("SELECT * FROM customdata WHERE Tunnus = ('$user') AND Salasana = ('$hashpass')");

//Toistaa ylempää kunnes pari löytyy (tai ei löydy)
while ($tietue = mysql_fetch_array($kysely, MYSQL_ASSOC)) {

      //Muunnetaan tietokannan taulujen nimet PHP -muuttujiksi
      $db_user = $tietue["Tunnus"];
      $db_pass = $tietue["Salasana"];

      //Suoritetaan tunnusparien vertailu
      if ($user == $db_user && $hashpass == $db_pass)
 	      echo "Tunnukset täsmäsivät!";
      else
              echo "Tunnukset eivät täsmänneet!";
}


//Suljetaan yhteys
mysql_close($yhteys);

?>

Koodi toimii muuten, mutta if -tarkistuksen else -ehto ei koskaan tulostu, vaikka tunnuspari olisi väärä. Jos tunnuspari on väärä niin ei tulostu ruutuun mitään...

Toivonmukaan myös arvostelette koodin, ja kerrotte miten sitä voisi parantaa esim. turvallisuuden suhteen...

Valitset taulusta vain ne käyttäjätiedot, jotka täsmäävät. Jos siis tiedot ovat väärät, yhtään taulukon riviä ei tule valittua, jolloin myöskään while-silmukkaa ei suoriteta kertaakaan.

<?php
if ($tietue = mysql_fetch_array($kysely, MYSQL_ASSOC)) {
  echo "Löytyi.\n";
  if ($tietue = mysql_fetch_array($kysely, MYSQL_ASSOC)) {
    echo "Löytyi toinenkin! Kannassa taitaa olla virhe.\n";
  }
} else {
  echo "Ei löytynyt.\n";
}
?>

Voisi olla järkevää ottaa SQL-kyselystä salasana pois, jolloin saisi kannasta oikean käyttäjän tiedot. Jos mitään ei löydy, tulostetaan "tuntematon käyttäjä", ja väärän salasanan tapaus voidaan käsitellä erikseen.

Tuota... Eikös tuo ensin hae tuolla kyselyllä kaikki ne rivit tietokannasta, joissa tunnus on annettu tunnus ja salasana on annetun salasanan md5-hash? Sitten siinä käydään läpi nämä löytyneet rivit, joita on oletettavasti joko 0 (tunnusta ei löydy) tai 1 (tunnus löytyy) kappaletta. Toisin sanoen koko ehtolausetta ei koskaan edes voida suorittaa väärällä tunnus-salasana-parilla.

If tarkistuksen else -ehto ei koskaan tulostu koska sinä haet elsesi vastaisella ehdolla mysql tietokannasta tietoa, joten elsen mukaista tulosta ei tule ikinä tuolla koodilla. Sama asia, kuin "Jos 1 on 1 niin .. Tai jos 1 on 0 niin .."

$kyselyn sisällähän on kaikki rivit, joissa käyttäjänimi ja salasana täsmäävät, eli niitä on joko 1 tai 0. Sinä haet riviä jossa tunnus ja salasana ovat oikein. Elikkä jos käyttäjänimi ja salasana ovat oikein, niin rivien määrä on 1, muussa tapauksessa 0.

Itse toteuttaisin tuon niin, että hakisin mysql -tietokannasta kaikki rivit jotka täsmäävät käyttäjänimeen, ja sitten vertaisin käyttäjän syöttämää salasanaa tietokannassa olevaan salasanaan. Molempi parempi.

Turvallisuuden kannalta sen verran, että kannattaa käyttäjän rekisteröityessä muuttaa erikoismerkit <, >, ! ja ? vaikkapa tällä käyttäjäystävällisempään muotoon, tai sitten jos käyttäjänimi sisältää jonkin noista niin ei anneta rekisteröityä.

No tein teidän vinkkien pohjalta tälläisen koodin, ja nyt se sitte toimiikin:

<?php

//Muodostetaan yhteys ja valitaan tietokanta
$yhteys = mysql_connect("localhost","root","******");
mysql_select_db("customdata",$yhteys);

//Tuodaan tunnus ja salasana -tiedot turvallisesti sovellukseen ja hashataan salasana
$pass = md5($_POST['pass']);
$user = $_POST['user'];

//MySQL -kysely: Etsii oikeita pareja tietokannasta
$kysely = mysql_query("SELECT * FROM customdata WHERE Tunnus = ('$user') AND Salasana = ('$pass')");

//Tarkistetaan tunnusparit
if (mysql_fetch_array($kysely,MYSQL_ASSOC) == true)
        echo "Tunnus täsmäsi!";
else
        echo "Tunnus ei täsmännyt!";


 //Suljetaan yhteys
mysql_close($yhteys);

?>

Suosittelen vakavasti ottamaan käyttöön UTF-8 koodauksen ja sitten käyttäjätunnus sekä salasana seuraavasti:

<?php

if (ctype_alnum($username) && ctype_alnum($password)):
 $kysely = mysql_query("SELECT * FROM customdata WHERE Tunnus = ('$user') AND Salasana = ('$pass')");

    //Tarkistetaan tunnusparit
    if (mysql_fetch_array($kysely,MYSQL_ASSOC) == true):
        echo "Tunnus täsmäsi!";
    else:
        echo "Tunnus ei täsmännyt!";
    endif;

endif;

?>

Tuo mysql_close() metodi on turha, PHP sulkee yhteyden automaattisesti. Nähtävästi voit kuitenkin tarvita tuota yhteyttä myöhemmin sivullasi joten on ihan turha availla ja sulkea yhteyksiä jatkuvasti, ne tuottavat turhia viiveitä.

--W--

Mitä toi ctype_alnum -funktio oikein tekee?

Funktio tutkii, muodostuuko merkkijono pelkistä numeroista ja (englannin kielen?) aakkosista.

Manuaalia lainatakseni: ctype_alnum testaa a-z, A-Z ja 0-9 merkit.

Hyvin yleinen kun tehdään käyttäjäkirjautumisia, yleensäkin ottaen ctype_ -metodit ovat hyödyllisiä sekä lisäksi sitten htmlspecialchars ja _encode metodit.

-W-

Itse olen ainakin toteuttanut mielestäni helpomman (en sitten tietoturvatasosta tiedä) näin:

<?php

$username = $_POST["username"];
$password = md5($_POST["password"]);

$query = mysql_query("SELECT password FROM users WHERE username = '$username'");
$oikea_passu = mysql_result($query, 0, 0);

if($oikea_passu == $password) {
echo "OK";
}
else {
echo "Fail!";
}

?>

Ei oo sisennyksiä kun kirjotin suoraan tähän.

punppis kirjoitti:

Itse olen ainakin toteuttanut mielestäni helpomman (en sitten tietoturvatasosta tiedä) näin:

<?php

$username = $_POST["username"];
$password = md5($_POST["password"]);

$query = mysql_query("SELECT password FROM users WHERE username = '$username'");
$oikea_passu = mysql_result($query, 0, 0);

if($oikea_passu == $password) {
echo "OK";
}
else {
echo "Fail!";
}

?>

Ei oo sisennyksiä kun kirjotin suoraan tähän.

Vastaus tietoturvaan:

SQL INJECTION