1

The session is not passing and I want to restrict the users from viewing the login page while they are logged in for that I tried many things, but it didn't work:

My login page

<?php
    error_reporting(E_ALL);
    ini_set('display_errors', 1);

    require_once('connect.php');
    extract($_POST);

    $result = mysqli_query($link, "SELECT * FROM users ");

    $row = mysqli_fetch_assoc($result);
    //var_dump($row['username']);
    //var_dump($row['password']);
    if(isset($_POST['login'])){
        $username = $_POST['username'];
        $password = md5($_POST['password']);
        if ($username == $row['username'] && $password == $row['password']){
            session_start();
            $_SESSION['nID'] = true;

            //echo"Login";
            header('Location: home.php');
        } else {
            echo"Login failed";
        }
    }
?>
<!DOCTYPE html>
<!--
    To change this license header, choose License Headers in Project Properties.
    To change this template file, choose Tools | Templates
    and open the template in the editor.
-->

<html>
    <head>
        <meta charset="UTF-8">
        <title>Login page</title>
        <link href="style.css" type="text/css" rel="stylesheet">
    </head>

    <body>
        <div id="frm">

            <form action="login.php" method="POST" style="width: 232px; padding-left: 490px;">
                <h1> Login</h1>
                <p>
                <label>Username</label>
                <input type="text" id="username" name="username" />
                </p>

                <p>
                <label>password</label>
                <input type="password" id="password" name="password"/>
                </p>
                <p>
                <input type="submit" id="btn" value="login" name="login" style="border-radius: 30%; background-color: gold; box-shadow: 0 12px 16px 0 rgba(0,0,0,0.24), 0 17px 50px 0 rgba(0,0,0,0.19);"/>
                </p>
                <p>
                Not yet a member <a href="register.php">Register here</a>
            </form>
        </div>
    </body>
</html>

My home page

<?php
    session_start();
    if ($_SESSION['nID'] == false) {
        header("Location: login.php");
        die();
    } elseif ($_SESSION['nID'] == true) {
        header("Location: Home.php");
        die();
    } else {
        echo"cant connect";
    }
?>
<html>
    <head>
        <link href="bootstrap-3.3.7-dist/css/bootstrap.min.css" rel="stylesheet">
    </head>

    <body>
        <ul class="nav nav-pills">
          <li role="presentation" class="active"><a href="welcome.php">Home</a></li>
          <li role="presentation"><a href="info.php">Information</a></li>
          <li><a href="logout.php">Logout</a>
        </ul>

         <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.12.4/jquery.min.js"></script>
            <!-- Include all compiled plugins (below), or include individual files as needed -->
            <script src="bootstrap-3.3.7-dist/js/bootstrap.min.js"></script>
    </body>
</html>

The session is not passing and it doesn't prevent the user from viewing the homepage while they aren't logged in.

I have tried many different things, but nothing seems to work.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Irtaza Rawjani
  • 101
  • 3
  • 13
  • 2
    you haven't added session_start(); at top of page. – Th3 Jul 10 '17 at 09:28
  • 3
    why restrict? why not just redirect if `$_SESSION` isset? – treyBake Jul 10 '17 at 09:29
  • Add top of the page and no need to start session in home page. And check you `session` using `var_dump()` in your home page. – Jaydeep Mor Jul 10 '17 at 09:29
  • I can see `$_SESSION['nID'] = true` in the first page, but I can't see `$_SESSION['nID'] = false`. Most likely in that case the value simply isn't set. Not sure that will evaluate to `false` necessarily in order to cause the redirect. – ADyson Jul 10 '17 at 09:29
  • @JaydeepMor session needs to be started in every page that uses it, unless that page has a "require" statement calling another script where the command is already issued. – ADyson Jul 10 '17 at 09:30
  • This looks very strange, you get all rows from the user-table and then check if your credentials match with the first found row. That will only work if you only have one user. And using md5 to hash the password is also a very bad idea. – jeroen Jul 10 '17 at 09:35
  • ***WARNING*** Do ***not*** use [`extract()`](http://php.net/manual/en/function.extract.php) on untrusted data such as `$_POST`. If you do make sure you use one of the non-overwriting flags values such as EXTR_SKIP and be aware that you should extract in the same order that's defined in variables_order within the php.ini. – Martin Jul 10 '17 at 09:36
  • 3
    The quality of answers to this question (so far) is shockingly low. `:-(` – Martin Jul 10 '17 at 09:42
  • 2
    ___Simple Rule___ Run your `session_start()` as the first command after the first ` – RiggsFolly Jul 10 '17 at 09:57
  • The `var_dump()` commands would also totally destroy you ability to successfully run a `header()` command as well, as any output to the browser before attempting to send any headers destroys the ability to send headers. They must be the first things sent to the browser – RiggsFolly Jul 10 '17 at 09:58
  • 3
    Please dont __roll your own__ password hashing. PHP provides [`password_hash()`](http://php.net/manual/en/function.password-hash.php) and [`password_verify()`](http://php.net/manual/en/function.password-verify.php) please use them. And here are some [good ideas about passwords](https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet) If you are using a PHP version prior to 5.5 [there is a compatibility pack available here](https://github.com/ircmaxell/password_compat) – RiggsFolly Jul 10 '17 at 10:00
  • Please do not update your question with code from the answers below, it is very confusing for people to read your question when the contents is infact the answer!! – Martin Jul 10 '17 at 11:41
  • If you wish to show changes you've made to your code since the original question, please **edit** your question and post the changed code BELOW the current question code, with a header stating that it's an update. Thank you. – Martin Jul 10 '17 at 11:42

6 Answers6

6

Some thoughts on this question:

  • 1) Stop using extract(). You simply don't need it.

    Warning
    Do not use extract() on untrusted data, like user input (i.e. $_POST, $_FILES, etc.). If you do, for example if you want to temporarily run old code that relied on register_globals, make sure you use one of the non-overwriting flags values such as EXTR_SKIP and be aware that you should extract in the same order that's defined in variables_order within the php.ini.

    From the Manual.

  • 2) As noted in another answer Your SQL query is far too vague; you're returning the first answer of a search of the whole DB rather than searching for any specific criteria.

    SELECT password FROM users WHERE username=username_here LIMIT 1
    

    And then take this row and compare with the given password:

    if($password === $row['password'])
    
  • 3) Your password system used on MySQL / PHP is NOT GOOD ENOUGH. Stop using md5() and employ password_hash and password_verify PHP functions. Please read how to do it properly and this comment.

  • 4) Every time you use header("Location: ...") to redirect the user it is highly recommended you add a die or exit command immediately afterwards in order to cease the code execution on the current page. For example:

    header("Location: this_page_will_never_load.php");
    header("Location: this_page_will_always_load_instead.php");
    
  • 5) require and include functions do not require brackets.


NOTE

Re the numerous answers here referencing session_start(); if session_start() is called after output is sent to the browser, then there will be an error notice generated. OP has not reported an error notice even with:

   error_reporting(E_ALL);
   ini_set('display_errors',1);

So session_start() placement in the code is not an issue in this specific situation.

However:
It is best practise to put your session_start() as early as possible in your code and before such debug things as var_dump which would cause session_start not to load becase var_dump has already thrown data out to the browser.


Finally, an answer to your problem:

I want to restrict the users from viewing the login page while they are logged in for that I tried many things but it didn't work:

Your code in login.php:

   if(isset($_POST['login'])){
        ///session stuff etc. 
    }

The above code on your login.php page will only execute if the page is being given POSTed data. What you have is that once someone is logged in correctly and they then return to the login.php page, they are not resubmitting the POSTed data so this code block is simply not running.

Because this code block contains all your $_SESSION references this is why it looks like $_SESSION is not running.

Instead you want to do this (simplified) in login.php:

session_start();
if(isset($_POST['login'])){
    // setup session values, 
    // once POSTed login data is checked and authorised in the database
    $_SESSION['nID'] = true;
}
elseif ($_SESSION['nID'] === true){
     // is already logged in so redirect to the index page. 
     header("Location: index.php");
     exit;
}
else {
     // this fires if no POSTed data is sent and no valid 
     // session is found. 
}
Martin
  • 22,212
  • 11
  • 70
  • 132
1

Try this condition in your home.php file:

session_start();
if (!isset($_SESSION['nID']) || empty($_SESSION['nId'])) {
   header("Location: login.php");
   die();
}
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Akshit Ahuja
  • 337
  • 2
  • 15
  • 2
    I am pleased to see at least one person is correctly using a `die` statement after a header redirect. However this answer as a whole is more of a comment than an actual answer. – Martin Jul 10 '17 at 09:39
-1

You try this code:

<?php
    session_start();

    error_reporting(E_ALL);
    ini_set('display_errors', 1);

    require_once('connect.php');
    extract($_POST);

    $result = mysqli_query($link, "SELECT * FROM users ");

    $row = mysqli_fetch_assoc($result);
    //var_dump($row['username']);
    //var_dump($row['password']);
    if(isset($_POST['login'])){
        $username = $_POST['username'];
        $password = md5($_POST['password']);
        if ($username == $row['username'] && $password == $row['password']){
            //session_start(); removed it
            $_SESSION['nID'] = true;

            //echo"Login";
            header('Location: home.php');
        } else {
            echo"Login failed";
        }
    }
?>

On every page, you need to add session_start() in the page heading.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Ricky
  • 50
  • 9
  • need add at top of page, login page didn't add at top of page – Ricky Jul 10 '17 at 09:31
  • 2
    why does it need to be at the top? It only needs to be present before any attempt to use the session is made, which it is. – ADyson Jul 10 '17 at 09:32
  • Because, it must be set before content is sent to output like a call to `header` function. – ADreNaLiNe-DJ Jul 10 '17 at 09:33
  • 1
    @ADreNaLiNe-DJ no mention of that in the manual http://php.net/manual/en/function.session-start.php – ADyson Jul 10 '17 at 09:34
  • @ADyson the manual doesn't refer to this, but try to make an `echo` or something that outputs content, and you'll see a `headers already sent` error. – ADreNaLiNe-DJ Jul 10 '17 at 09:37
  • 1
    @ADreNaLiNe-DJ but look at OP's code, they aren't sending any content beforehand. You may be right about sending content, but that isn't relevant here and doesn't imply that the command must be the first thing in the script. – ADyson Jul 10 '17 at 09:39
  • @ADyson You don't know that. As you don't know what's happening in `require_once("connect.php")`. – ADreNaLiNe-DJ Jul 10 '17 at 09:41
  • 1
    @ADreNaLiNe-DJ no but it's pretty reasonable to assume it just creates a DB connection. And OP is not reporting any header errors, instead the desired redirect just doesn't happen, which is more to do with the faulty logic in the code. – ADyson Jul 10 '17 at 09:42
  • @ADyson `reasonable` is not a factand when you are trying to find where the error is, you can not base your reasoning on `reasonable` assumptions. ;o) – ADreNaLiNe-DJ Jul 10 '17 at 09:45
  • 2
    @ADyson does have a point, if a `session_start()` is called after output is sent to the browser, then there will be an error notice generated. OP has not reported an error notice so `session_start` placement in the code is not an issue in this specific situation. – Martin Jul 10 '17 at 09:48
  • @Ricky you've just expanded the code sample a bit, it doesn't address the point that simply moving session_start() around doesn't solve the problem, since it was already included in an acceptable place. – ADyson Jul 10 '17 at 09:59
-1

First: First of all, your query is wrong. You're always checking the value with the first user in the table. You need to a query with the where clause.

SELECT * FROM users WHERE username=username_here AND password=hash_password_here

Second: Your If statement should be like the following.

<?php
    session_start();
    if (!isset($_SESSION['nID'])) {

       header("Location: login.php");
       die();
    }
?>

Third: Try to use prepared statements to avoid an SQL injection.

$stmt = $link->prepare("SELECT * FROM users where username=? and password=?");

$stmt->bind_param('ss', $username, md5($password));

$stmt->execute();
$get_result = $stmt->get_result();

$row_count = $get_result->num_rows;

if ($row_count > 0) {

    session_start();
    $_SESSION['nID'] = true;

    header('Location: home.php');
    die();
}
else {
    echo"Login failed";
}

4th: Don't use Md5() for passwords. Try to use password_hash() and password_verify(). Reference link.

While registrating, use password_hash() to hash the password and store it in the database and while logging in, use password_verify() to verify the password like this. Reference link.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
JYoThI
  • 11,977
  • 1
  • 11
  • 26
  • Unless I am missing something, this will enter an infinite reload loop of the home page. – jeroen Jul 10 '17 at 09:38
  • not working shows the error that This page isn’t working localhost redirected you too many times. Try clearing your cookies. ERR_TOO_MANY_REDIRECTS but when i remove session_start(); – Irtaza Rawjani Jul 10 '17 at 09:38
  • First of all your query is wrong . your always checking the value with first user . you need to query with where clause – JYoThI Jul 10 '17 at 09:41
  • Downvoter can you please explain why it deserve downvote – JYoThI Jul 10 '17 at 09:44
  • Maybe you missed the first 2 comments below your answer? I wonder why it was upvoted :-) – jeroen Jul 10 '17 at 09:44
  • I didn't downvote but you're still going to cause an infinite redirect loop by repeated redirects to Home.php which is already the page that the script is written in – ADyson Jul 10 '17 at 09:44
  • 2
    Sorry to be pernickerty, @JYoThI but hashes of passwords will not be the same, so you need to take the hash out of the DB and then compare it wth the posted hash via `password_hash`. You can't compare in the DB for password hashes directly. – Martin Jul 10 '17 at 09:45
  • i think he didn't used password_hash .he just used md5 only .that's why i suggested like that .@Martin – JYoThI Jul 10 '17 at 09:49
  • 1
    @JYoThI That's actually true, however note that you are introducing an sql injection problem here. – jeroen Jul 10 '17 at 09:51
  • did you using password_hash or just Md5() ? @ – JYoThI Jul 10 '17 at 09:55
  • 1
    to make it really complete, adding `die();` after the `header()` call would be sensible. And then I think we finally have a decent solution :-) – ADyson Jul 10 '17 at 10:01
  • everything is updated thans all , and still it deserved downvote ? why ? – JYoThI Jul 10 '17 at 10:04
  • it shows this error Undefined variable: password in C:\xampp\htdocs\login\home.php on line 6 – Irtaza Rawjani Jul 10 '17 at 10:19
  • JYoThI please edit the files and re edit all the additions – Irtaza Rawjani Jul 10 '17 at 10:20
  • 1
    did you using password_hash or just Md5() ? @IrtazaRawjani – JYoThI Jul 10 '17 at 10:20
  • No i didnt i dont know how to so please re edit it so i can use it – Irtaza Rawjani Jul 10 '17 at 10:23
  • @IrtazaRawjani you would have to use password_hash when adding the users to the DB too, so if you didn't do this already, some other code would need altering, and all your existing passwords to be reset. MD5 is not secure anymore. – ADyson Jul 10 '17 at 10:25
  • Follow my fourth step . there i gave reference link . try to read it . and also don't forgot to read @Martin answer – JYoThI Jul 10 '17 at 10:26
  • i am making a simple login system so i really dont want it tobe highly secure – Irtaza Rawjani Jul 10 '17 at 10:28
  • 2
    @IrtazaRawjani, Sorry but this is entirely the wrong approach for learning PHP life becomes much easier if you do things the best way ***from the outset***. Imagine trying to learn a new spoken language but not bothering to learn the verbs because you don't feel you need them at the start. You can begin by saying what you want "food!" but it's not the same as being able to say "what food can I buy", etc. – Martin Jul 10 '17 at 10:32
  • "i really dont want it tobe highly secure". Interesting approach! Simplicity is good, but just think, if you make a really good login system now, it might be re-usable in future. And the suggested changes don't really introduce that much complexity anyway. And then you learned something useful instead of doing something just-about-good-enough that doesn't prepare for the future. I'd also be interested if I was your client, hearing you tell people you don't want the system to be as secure as it could be. – ADyson Jul 10 '17 at 10:54
  • i have used password hash but it is still not working – Irtaza Rawjani Jul 10 '17 at 10:56
  • Parse error: syntax error, unexpected ',' in C:\xampp\htdocs\login\home.php on line 6 this is the error it shows – Irtaza Rawjani Jul 10 '17 at 11:04
  • it's my old answer . read my answer here . you will get idea . https://stackoverflow.com/questions/44198162/when-i-go-to-localhost-and-try-the-username-and-the-password-it-tells-me-that-is @IrtazaRawjani – JYoThI Jul 10 '17 at 11:12
-2

You need to add session_start(); on every page to get the session variables.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Akshit Ahuja
  • 337
  • 2
  • 15
-2

You have to call the session_start() function in the file where you are trying to use a session variable.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Raghav salotra
  • 820
  • 1
  • 11
  • 23
  • Code written only shows session being set. Since you have set the session and it has not been cleared post that. So you should have code which destroys the session and then you should not be able to view the home.php if session is not set again. session_destroy() should be called on logout. – Raghav salotra Jul 10 '17 at 09:40
  • True about destroying the session, it's good practise but it's not relevant to the question, nor does it justify your answer so not sure why you mentioned it. – ADyson Jul 10 '17 at 09:50