current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote -6 down vote favorite

I'm trying to figure out the best way to manage user authentication.

At the moment I'm using this way

include("Conection.php");
$usuario = $_POST["Nick"];
$contra = $_POST["Pass"]; 
$sql = "SELECT * FROM usuario WHERE Nick = '$usuario' AND Password = '$contra'"; 
$result = $conn->query($sql);
$row = $result->fetch_assoc();
if ($result->num_rows == 0) { 
      echo '<script languaje=javacript>
    alert ("Inicion de sesion rechazada")
    window.location="../Entrar.html"
    </script>';
} 
else 
{ 
     // Inicias la sesion 
     session_start(); 
     $_SESSION['Usuario'] = $row['Nick']; 
     $_SESSION['estado'] = 'Autenticado'; 
     echo ("<script>location.href='../../Index.php'</script>");
     // Muestras el contenido de la pagina 
}

and if my user logs in he will have different menu options

<?php
    session_start(); 
    if(isset($_SESSION['Usuario']) and $_SESSION['estado'] == 'Autenticado')                    { ?>
    <a href="Navegacion/Entrar.html">Logeado</a>&nbsp;&nbsp;&nbsp;
<?php } 
    else 
    {    ?>
    <a href="Navegacion/Entrar.html">Entrar</a>&nbsp;&nbsp;&nbsp;
    <a href="Navegacion/Registrarse.html">Registrarse</a>&nbsp;&nbsp;&nbsp;
    <a href="Navegacion/Carrito.html"><img src="Images/Carrito.png" alt=""  width="20" height="20" ></a>
<?php   }    ?>

I know this way it's way weaker with SQL injections and stuff like that.

My main question was, Which it's the best way to protect againts sql injections, seems that the answer it's PDO, since I can not use comments and let me thank here all you that help me, thank you mdfst13

share|improve this question

put on hold as unclear what you're asking by forsvarir, t3chb0t, Tunaki, Vogel612, alecxe Mar 4 at 16:42

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question.If this question can be reworded to fit the rules in the help center, please edit the question.
3  
You already know that you have an SQL injection problem. Why don't you fix it instead of asking us to review code that you already know is problematic? – 200_success♦ Mar 3 at 17:57
    
You also seem to be confusing the use of sessions with authentication/login state. These are really two different things (though there are relations between the two when it comes to working with sessions securely). Also, why javascript redirections? Try looking at phptherightway.com to focus properly on the fundamentals. – Mike Brant Mar 3 at 20:31
up vote 1 down vote

Avoiding SQL injection

$usuario = $_POST["Nick"];
$contra = $_POST["Pass"]; 
$sql = "SELECT * FROM usuario WHERE Nick = '$usuario' AND Password = '$contra'";

The smallest change to this to avoid SQL injection would be something like 

$usuario = $conn->real_escape_string($_POST['Nick']);
$contra = $conn->real_escape_string($_POST['Pass']);
$sql = "SELECT * FROM usuario WHERE Nick = '$usuario' AND Password = '$contra'";

But modernly best practice is to use something like PDO with bind variables. You don't include enough context to make that change here.

I also changed double quotes to single quotes because I prefer them for static strings. I only use double quotes for interpolated strings like $sql.

PHP vs. Javascript

      echo '<script languaje=javacript>
    alert ("Inicion de sesion rechazada")
    window.location="../Entrar.html"
    </script>';

First, consider using a NOWDOC for a multiline string. E.g.

    echo <<<'EOSCRIPT'
<script languaje=javacript>
  alert ("Inicion de sesion rechazada")
  window.location="../Entrar.html"
</script>
EOSCRIPT;

But you generally don't want to do things like a redirect in Javascript. Doing it with HTML headers is more reliable. 

    header('Location: ../Entrar.html');

Note that this should be an absolute URL, but I don't know which one.

You have to emit headers before any HTML, even the DOCTYPE.

share|improve this answer

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