0

I'm currently building a very, very basic login script that basically connects to the database, and checks to see if the value the user enters in a form matches up. For some reason, however, my code REFUSES to do anything when the submit button is clicked!!?? Here's the code...

<?php
mysql_connect("localhost", "username", "password");
mysql_select_db("dbname");
?>

<body>
<?php
if(isset($_POST['submit'])){
$username = $_POST['username'];
$password = $_POST['password'];

$result = mysql_query("SELECT * FROM users WHERE username='$username' AND password='$password'");
$num = mysql_num_rows($result);
if($num == 0){
echo "Bad login, go <a href='login.php'>back</a>.";
}else{
session_start();
$_SESSION['username'] = $username;
header("Location: admin.php");
}


}else{
?>


<form action='login.php' method='post'>
   Username: <input type='text' name='username' /><br />
   Password: <input type='password' name='password' /><br /><br />
   <input type='submit' value='Login' />
</form>
<?php
}
?>
</body>

Any ideas?

Widor
  • 12,075
  • 6
  • 35
  • 60
Adam McArthur
  • 870
  • 12
  • 25
  • echo the query string before you run it so you can see if it looks right. and output mysql_error too. – dnagirl May 28 '12 at 14:23

8 Answers8

7

You are checking for

if(isset($_POST['submit'])){

...but $_POST['submit'] will never be set, because you didn't give any of your form controls a name="submit" attribute.

Change the submit button to this:

<input type='submit' name='submit' value='Login' />

Please also sanitise your database input, preferably by means of parameterised queries.

DaveRandom
  • 84,004
  • 11
  • 142
  • 168
  • 3
    Just to second the request to [escape your database input](http://bobby-tables.com/). This is really important. – fredley May 28 '12 at 14:25
  • Or just simply `if ($_POST)`. – flowfree May 28 '12 at 14:26
  • @bsdnoobz `if (!empty($_POST))` is better as this also has an implicit `isset()` (although thinking about it I'm not sure if it's necessary) – DaveRandom May 28 '12 at 14:31
  • Rather than escaping input, use prepared statements. http://bobby-tables.com/php – Andy Lester Sep 22 '13 at 22:07
  • @AndyLester That was supposed to be implicit, but I have updated the answer to use better phrasing as "escape" does have a suggestion of manipulating strings instead of separating the parameters from the query. – DaveRandom Sep 22 '13 at 22:15
  • @DaveRandom: Someone who is new enough to programming to not know about SQL injection isn't going to be helped much by implicit. Thanks for updating. – Andy Lester Sep 22 '13 at 23:13
1

Easy one, try

<input type='submit' name='submit' value='Login' />

instead of

<input type='submit' value='Login' />

Hope this helps!

EDIT: You have not named submit

Justin T.
  • 3,503
  • 1
  • 20
  • 42
1

the answer given by DaveRandom seems the most valid to me.

However you have several failures in your code that need to be taken care of first.

You don't sanitize your input, which can be fatal, especially in a login form.

use

  mysql_real_escape_string($_POST['username']));
  mysql_real_escape_string($_POST['username']));

and take other measures to avoid SqlInjection.

And is your form redirecting to the correct page? If the name of the page where it relies is login.php it should work, but you could also use

 <form action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post">

Anyway, I don't think it's good practice to process in the self page, as you should always redirect to a handler page instead.

João Dias
  • 1,078
  • 1
  • 13
  • 38
0

Are you posting to the same page as the form?

If you are, try removing the action="login.php".

CharliePrynn
  • 2,784
  • 5
  • 35
  • 63
0
  1. Using header(..) upon successful login will not work. This is because a few character have might already have been sent out to the browser. You need to ensure that no characters are sent out. Somewhat this way:

    all other HTML stuffs here ..
  2. Change if(isset($_POST['submit'])) to if(isset($_POST['username'])). This is because you submit input does not have a 'name' field. Test against something that has.

  3. As pointed out by others, you script is open to SQL injection. Fix it by either using

Community
  • 1
  • 1
UltraInstinct
  • 38,991
  • 11
  • 73
  • 100
0

This is not right, your session will suffer since youve allready output some contents.

Your action="login.php" must point to this file, e.g. yor sample script is saved in login.php and it'd be fine. And as stated in other answers, you must set name="something" to access it through $_POST['something']

<?php // some php scripting ?>

<!-- blank lines also counts as output --> 
<body>
  <?php session_start(); header("What ever"); ?>
</body>
mschr
  • 8,221
  • 3
  • 19
  • 35
0

And also is recommended to put session_start() right on the top of the code.

So try

<?php session_start(); ... ?>
MelkorNemesis
  • 3,265
  • 2
  • 18
  • 20
0

The implementation of login page (php-mysql) is shown below :

<?php
if (!isset($_SESSION)) {
session_start();
}

$loginFormAction = $_SERVER['PHP_SELF'];
if (isset($_GET['accesscheck'])) {
   $_SESSION['PrevUrl'] = $_GET['accesscheck'];
}

if (isset($_POST['username'])) {  //from form username textbox
   $loginUsername=$_POST['username'];
   $password=$_POST['password'];
   $MM_fldUserAuthorization = "access_level"; // from db user access-level
   $MM_redirectLoginSuccess = "login_success_page.php";
   $MM_redirectLoginFailed = "login_failed_page.php";
   $MM_redirecttoReferrer = false;
   mysql_select_db($DB_NAME, $DB_LINK);

   $Login_query=sprintf("SELECT username,password, access_level FROM tbl_user WHERE username=%s AND password=%s",
   GetSQLValueString($loginUsername, "text"), GetSQLValueString($password, "text"));

   $Login = mysql_query($Login_query, $DB_LINK) or die(mysql_error());
   $loginFoundUser = mysql_num_rows($Login);

   if ($loginFoundUser) {

      $loginStrGroup  = mysql_result($Login,0,'access_level');

      if (PHP_VERSION >= 5.1) {session_regenerate_id(true);} else {session_regenerate_id();}

      $_SESSION['MM_Username'] = $loginUsername;
      $_SESSION['MM_UserGroup'] = $loginStrGroup;

      if (isset($_SESSION['PrevUrl']) && false) {
         $MM_redirectLoginSuccess = $_SESSION['PrevUrl'];
      }
      header("Location: " . $MM_redirectLoginSuccess );
   }
   else {
      header("Location: ". $MM_redirectLoginFailed );
   }
}
?>
OSezer
  • 41
  • 4
  • Could you explain your answer please? That'll help others in similar situations to understand what you've done to apply it in their cases. – Wai Ha Lee Apr 01 '16 at 20:58
  • Firstly, session is created (session_start()). Then If 'username' in the form is set (isset($_POST['username'])==true), username and passwords from forms are querying with Sql Query ("SELECT username...WHERE username=username from form "). If there is user in table, accesslevel is also controlled. Then,if it is successful, page is redirectted to login success page. If there is trouble, it goes to failed page. – OSezer Apr 01 '16 at 21:22