User registration

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

User avatar
Stryks
Forum Regular
Posts: 746
Joined: Wed Jan 14, 2004 5:06 pm

Re: User registration

Post by Stryks »

Well .. there's a fine line between helping you and me writing your entire app. We're coming close to that I feel.

So, this is your registration code re-written more along the lines of what I would have done. This is by no means perfect, and could certainly use some honing, but I've stayed as close to your original code as possible so that you can see how to convert your login page to match.

Code: Select all

<?php
// Never hurts to keep sessions around in case you need them
session_start();
 
// If form data is posted, run processing script
if ($_SERVER['REQUEST_METHOD'] == 'POST') {
    //include function files for this application
    require("PDMS_fns.php");
 
    // Check email
    if (!(isset($_POST['email']) && valid_email($_POST['email']))) $error['email'] = 'Not a valid email address';
    // check username length
    if(!isset($_POST['username']) || (strlen($_POST['username']) < 6 ) || (strlen($_POST['username']) > 16)) $error['username'] = 'Username must be between 6 to 16 characters';  
    // check that passwords are OK
    if((isset($_POST['password']) && isset($_POST['password2'])) {
        if($_POST['password'] !== $_POST['password2']) $error['password'] = 'Passwords do not match';
        if (strlen($_POST['password']) < 6 || strlen($_POST['password']) > 16) $error['password'] -'Password must be between 6 to 16 characters';   
    }  $error['password'] = 'Password data not found.  Possible form error.';
 
    // Proceed with processing if no errors have occurred
    if(!isset($error)) {
        // Connect to database
        $conn = db_connect($server, $username, $password) or die('Unable to connect to the database');
        // Select database
        mysql_select_db('database_name', $conn) or die('Could not select database.');
        // make sure user is unique - notice we have used mysql_escape_string to help secure the user input
        $username = mysql_escape_string($_POST['username']);
        $result = mysql_query("select * from users where username='$username'");
        if($result) {
            if(mysql_num_rows($result) < 1) {
                // Passed all validation and checking, ready to insert
                $password = md5('secret_salt' . md5($_POST['password'])); 
                $email = mysql_escape_string($_POST['email']); 
                $result = mysql_query("insert into users values('', '$username', '$password', '$email')");
                if($result) {
                    // Smiles all round - we're done here so let's move on
                    header('Location: http://www.yoursite.com/login.php');
                    exit();
                } else $error['database'] = 'An unknown database error has occurred on insert';
            } else $error['username'] = 'Username already exists. Please select a different username'; 
        } else $error['database'] = 'An unknown database error has occurred on select';
    }
}
 
// If we made it this far, either this is a non-submit, or there was an error registering
// Either way, begin display of the form
include "header1.php";
?>  
 
<div id="content">
<?php
    if(isset($error)){
?>
  <div>
<?php foreach($error as $key=>$item) echo "<strong>$item</strong></br>"; ?>  
  </div>
<?php
    }
?>
  <div id="left" style="left: 0px; top: 0px">
    <ul>
      <li>Email address :</li>
      <li>Preferred username :</li>
      <li>Password :</li>
      <li>Confirm password :</li>
    </ul>
   </div>
   <div id="right">
    <p style="left: -1px; top: 0px"><font size="4.5px">REGISTER</font></p>
    <form action="register.php" name="register_form">
    <input type="text" size="20" name="email"<?php if(isset($_POST['email'])) echo " value = \"{$_POST['email']}\""; ?>><br><br>
    <input type="text" size="20" name="username"<?php if(isset($_POST['username'])) echo " value = \"{$_POST['username']}\""; ?>> (6 to 16 chars)<br><br>
    <input type="password" size="20" name="password"<?php if(isset($_POST['password'])) echo " value = \"{$_POST['password']}\""; ?>> (6 to 16 chars)<br><br>
    <input type="password" size="20" name="password2"<?php if(isset($_POST['password2'])) echo " value = \"{$_POST['password2']}\""; ?>><br><br>
    <input type="submit"  name="Submit" value="submit">
    <input type="reset" name="Reset" value="reset">
    </form>
    </div>
</div>
 
So, take a look at that, try and get the feel for what it is doing, and then try and apply that to the login page. Don`t worry too much if you make something that doesn't work .... it's just important that you try. I certainly wont be coding for you like this again, purely because it is better for you to learn by doing than it is to just be given the answers. I've done this purely to give you an example of good practice using your own code as a base.

As for the password. I'm recommending a process change. I would hash the password (and have done in the above code) before storing the password, and then hash the users login password and compare it to the stored hash. The process change comes from the fact that hashes cant be returned the original value. This means that you cannot tell the user their old password because not even you with full database access can tell what a users password is.

So your 'forgotten password' process would invlove randomly generating a password, hashing and saving it, then sending the user the unhashed version while you still have access to it. Then when the user gets the email, they can log in and are free to change it to whatever they like.

A simple ...

Code: Select all

md5($_POST['password']);
... might be sufficient, but I like to use a salted hash-in-hash, just for an added level of complexity.

Anyhow, I hope this helps. Let me know how you go with the new merged login page.

Cheers

[edit] This is entirely untested by the way ... so it may take a few fixes to get it to work. Let me know if it's not going too well for you.
pritam79
Forum Commoner
Posts: 65
Joined: Wed Mar 26, 2008 9:28 am

Re: User registration

Post by pritam79 »

Hi stryks,
When I login as 'user1' and enter 'user1's' data in a table and then logout and login again as 'user2' and add 'user2's' data into the same table, i get to see both user1 and user2 data in the same table in MySql as well as on the php page as if i inserted data for a single user. I donot know as to what the problem is? Any help?
User avatar
Stryks
Forum Regular
Posts: 746
Joined: Wed Jan 14, 2004 5:06 pm

Re: User registration

Post by Stryks »

This probably should have been in a new thread. How did the above code go for you by the way?

I would imagine that when you say "in mysql I can see" that you are referring to viewing the database through a tool, like phpMyAdmin, in which case, yes, you should be able to see everyone's data. Only you (should) have direct access to the database remember.

As for the users viewing the data through the webpage, well, I guess it comes down to what specific data you are pulling from the database as to how much they see. When you write the data to a table, each entry should have the users id (the autonumber froom the users table) specified to indicate that row belongs to that user. Then when the user views rows from that table ...

Code: Select all

$sql = "SELECT col1, col2 FROM yourtable WHERE user_id = " . $_SESSION['user_id'];
Or something like that.

Cheers.
pritam79
Forum Commoner
Posts: 65
Joined: Wed Mar 26, 2008 9:28 am

Re: User registration

Post by pritam79 »

Stryks wrote:How did the above code go for you by the way?
Your previous code didn't work. When i entered correct input data, it said "Password data not found. Possible form error." All my form data was correct
User avatar
Stryks
Forum Regular
Posts: 746
Joined: Wed Jan 14, 2004 5:06 pm

Re: User registration

Post by Stryks »

Well ... I did say it was untested. :)

Replace this block of code ...

Code: Select all

   // check that passwords are OK
    if(isset($_POST['password']) && isset($_POST['password2'])) {
        if($_POST['password'] !== $_POST['password2']) $error['password'] = 'Passwords do not match';
        if (strlen($_POST['password']) < 6 || strlen($_POST['password']) > 16) $error['password'] = 'Password must be between 6 to 16 characters';  
    } else $error['password'] = 'Password data not found.  Possible form error.';
The thing about code you will get on here is that it is provided as is. You need to have a good look through the code, figure out what it does, and come to understand it.

When you got that message, I would have hoped you would have looked at the code provided, seen the location of the error message you were getting, and thereby discovered the location of the problem. All I had done was to omit the 'else' directive, meaning that that error was thrown in every instance, instead of just in cases when the 'if' failed.

There was another small error, but that too would have been detected had you taken a good close look.

Seriously, give it a look over, try to understand it, and then try to replicate it into your login page and post it back. You could learn a lot by doing so.

Either way, hope this helped.

Cheers
pritam79
Forum Commoner
Posts: 65
Joined: Wed Mar 26, 2008 9:28 am

Re: User registration

Post by pritam79 »

I am thinking about considering the 'username' field of my 'users' table as the primary key of that table without any U_ID field, and have a 'username' (foreign key) field in all the remaining tables along with their individual id fields as their primary keys. So that i can execute queries for data manipulation depending on the 'username' field of those tables. Will it be viable to not have an id field in the tables for contacts, reminders, appointments etc and consider only the 'username' field because in my application i have considered username as distinct and have not allowed more than one user to have the same username.
User avatar
Stryks
Forum Regular
Posts: 746
Joined: Wed Jan 14, 2004 5:06 pm

Re: User registration

Post by Stryks »

I suppose you *could*. I wouldn't really advise doing so though.

Here are a few reasons.

Firstly, it doesn't really take any more effort. When the user logs in, get their user ID while checking their password and store it to the session on successful login. You then have the user id to use in whatever queries you like.

From a database level, I think that most databases are much faster at searching indexed integers than text, and given a username length of say, 16 characters max, it's going to cut down on database size by having a lot of int(11)s in your various tables.

Last but not least is visibility and security. If I said delete=23, it would be far less obvious than saying delete=fred. That kind of obvious link between users and actions can prompt people to do a little URL tinkering to see if they can work around your systems. Also, while it makes it easier for you to see what rows belong to what user, it also does the same for anyone who might compromise your database.

Cheers
pritam79
Forum Commoner
Posts: 65
Joined: Wed Mar 26, 2008 9:28 am

Re: User registration

Post by pritam79 »

[quote="Stryks"]
Replace this block of code ...

Code: Select all

   // check that passwords are OK
    if(isset($_POST['password']) && isset($_POST['password2'])) {
        if($_POST['password'] !== $_POST['password2']) $error['password'] = 'Passwords do not match';
        if (strlen($_POST['password']) < 6 || strlen($_POST['password']) > 16) $error['password'] = 'Password must be between 6 to 16 characters';  
    } else $error['password'] = 'Password data not found.  Possible form error.';
Hi stryks,
Your code worked fine and i am implementing that in the other pages as well. Thanks a lot for the help. :)
I have an option of allowing a user to change the login password. I have done so with your logic of processing with one page. Is it possible to display a "password successfully changed" message on the login page now that i am able to redirect a user to the login page after he changes the password?
pritam79
Forum Commoner
Posts: 65
Joined: Wed Mar 26, 2008 9:28 am

Re: User registration

Post by pritam79 »

Hi stryks,
Your one page processing of the registration process worked and therefore i tried doing the same with the login page. When i login, i am able to do so as the single user, and when i insert records the records get inserted in the tables but not separately for different users. And even when i view the records in mysql, the username field for the user who had logged in before inserting records is not inserted in mysql. The problem is that, after registering successfully, i am directed to the login page but after that a user is not identified in the entire application, and therefore all users are recognized as one. Also, im mysql, if i query as 'select * from users' , i am given a list of all users registered in the registration process, but if i say ' select * from contacts where username='user1', i am given an empty set.

This is the login page-

Code: Select all

<?php
session_start();
// If form data is posted, run processing script
if ($_SERVER['REQUEST_METHOD'] == 'POST') 
  {
    //include function files for this application
    require("PDMS_fns.php");
    if(!filled_out($_POST)) $error['form'] = '<center><font color="red">You have not filled the form completely</font></center>';
      if($_POST['username'] && $_POST['password'])  // they have just tried logging in
        {
          if(login($_POST['username'], $_POST['password']))
            {
              
             // if they are in the database, register the user id
              $valid_user = $_POST['username'];
              session_register("valid_user"); // echo "p";
            }
          else
                   // unsuccessful login
            $error['form'] = '<center><font color="red">You could not be logged in.<br>Check your username and/or password</font></center>';
             
        }
       if(check_valid_user())
        // header('Location: home.php');
          echo "logged in as $valid_user";
  
}      
 
include "header1.php";
?>  
<div id="content">
<?php
    if(isset($error)){
?>
<?php foreach($error as $key=>$item) echo "<strong>$item</strong></br>"; ?>  
  
<?php
    }
?>
<html>
<body>
<table style="width: 768px; height: 195px;"><form action="login.php" method="post">
    <tr>
        <td colspan="2" style="text-align: center; font-weight: 700; font-family: Arial;">Login to start</td>
    </tr>
    <tr>
        <td style="text-align: right; width: 341px; font-family: Arial;">Username : </td>
        <td style="text-align: left"><input type="text" size="20" name="username"<?php if(isset($_POST['username'])) echo " value = \"{$_POST['username']}\""; ?>></td>
    </tr>
    <tr>
        <td style="text-align: right; width: 341px; font-family: Arial;">Password : </td>
        <td style="text-align: left"><input type="password" size="20" name="password">&nbsp;<a href="forgot_form.php">forgot 
        password ?</a>&nbsp;&nbsp; <a href="register.php">not registered ?</a></td>
    </tr>
    <tr>
        <td colspan="2" style="text-align: center"><input type="submit" name="submit" value="Submit">
    <input type="reset" name="reset" value="Reset">
    </td>
    </tr></form>
</table>
</body>
</html>
</div>
 
User avatar
Stryks
Forum Regular
Posts: 746
Joined: Wed Jan 14, 2004 5:06 pm

Re: User registration

Post by Stryks »

There's a few things I'm not too sure about with that code. Personally, I'm not a big fan on the way the database query and $_POST value verification is hidden in those functions you have, but I'll assume that everything is on the up and up there (mysql_real_escape_string for $_POST variables apart from the password value which you'll be hashing instead).

First thing I'd try is to change the way you are checking to see if a form submission was made.

Code: Select all

     if($_POST['username'] && $_POST['password'])  // they have just tried logging in
This doesn't look overly reliable to me ... what is going to make $_POST['username'] return true? That's a serious question by the way ... I'm not sure. Also, with statements like these, the lack of containing parentheses can trip you up.

What's worse, it looks a bit redundant. I mean ... isn't that just a redo of this ...

Code: Select all

if(!filled_out($_POST)) $error['form'] = '<center><font color="red">You have not filled the form completely</font></center>';
???

And like I said, I'd personally recommend keeping the state checks and validation as a part of this page so that you can see exactly what is happening. This filled_out() function is probably not portable, so it's only useful on this page anyhow. Even if it is ... there is an extraordinarily small advantage to functionalizing this that I'd actually recommend duplication.

Here is the logic I'd probably be going with.

Code: Select all

// If form data is posted, run processing script
if ($_SERVER['REQUEST_METHOD'] == 'POST') {
    //include function files for this application
    require("PDMS_fns.php");
    // Check we have what we need
    if(isset($_POST['username']) && isset($_POST['password'])) {
        $username = mysql_real_escape_string($_POST['username']);
        $password = md5('secretsquirrel' . md5($_POST['password']));
        
        $login_info = login($username, $password);     // login() should return false on failure, or an array of required valus from the users record, especially user id
        if($login_info !== false) {
            // Login success - store what we need
            $_SESSION['user'] = array('login'=>$_POST['username'], 'id'=>$login_info['id']);
            
        } else $error['form'] = '<center><font color="red">You could not be logged in.<br>Check your username and/or password</font></center>';    
    } else $error['form'] = '<center><font color="red">You have not filled the form completely</font></center>';
}    
 
This would require some work being done to the login function, but it should be pretty easy work. In or out of a function, no big deal really. If you want to keek the hashing method in a central location, then create another function called password_hash() or something and call it separately in your code so that you can see that it has been done, as opposed to hiding it in the login function (as I assume you have done).

Also, is there any point in having the login check in the mix? I think you'd be better off putting a check in to see if a user is logged in, and if they are, just bounce them away from the login page altogether so that they wouldn't even see the form.

Hope this helps, and let me know if you need help adjusting the login function.
Post Reply