I have made a very basic registration page whereby hitting 'submit' will add the details into my Users table with no validation or anything.
What I want to do now is start setting the login/registration side of my site up so just a few questions I hope you can answer:
1. When I fill in my registration form I would like to perform validation and then authentication. I understand that I can pass the form values to authentication.php (or whichever file) and can do the checks there however is there a way I can do the validation first to make sure the form is filled in corectly AND maybe display a line on that page. Eg. if username is not filled in, it could just say 'please fill in a username' -- I'm thinking of using the 'or die' function and just making the user press back to refill the form, but is there a better way??
[I have a few more but I guess I should ask them when this one is done]
I would always argue that the best way of validating forms server side is to split the script in two.
logon_display.php - A template that displays the form.
logon_input.php - The script that receives the input, validates and verifys the logon credentials.
Doing it like this will enable you to keep the form and the script that actually logs the user on separate entities.
login_display.php may look something like this:
PHP Code:
<?php
/* disable notices as variables will be undefined on first display */
error_reporting(E_ALL ^ E_NOTICE);
session_start();
Your logon_input.php will process the form and validate it. If validation and authentication success, a sesion variable called authenticated is set to true and the user is redirected to the next page. If it fails, the appropriate messages are set and added to the session and the user is redirected back to the logon page.
PHP Code:
<?php
session_start();
$_SESSION = array(); // clear any old session data
/* check a username is present */
if ((! isset($_POST['username'])) || trim($_POST['username'] == '')) {
$_SESSION['formMessage'] = 'Error Processing Request';
$_SESSION['username']['msg'] = 'Username Cannot be Left Blank';
header('Location: logon_display.php');
exit;
}
/* check a password is present */
if ((! isset($_POST['password'])) || trim($_POST['password'] == '')) {
$_SESSION['formMessage'] = 'Error Processing Request';
$_SESSION['password']['msg'] = 'Password Cannot be Left Blank';
header('Location: logon_display.php');
exit;
}
if (authenticate($username, $password)) {
/* valid username and password */
$_SESSION['authenticated'] = true;
header('Location: next_page.php');
} else {
$_SESSION['formMessage'] = 'Invalid Username or Password';
$_SESSION['user']['value'] = $username;
header('Location: logon_display.php');
}
/* this function will authenticate the user, prehaps via a database
or a file. in this case i just put the username and password in an
array */
function authenticate($username, $password)
{
$users = array ('user1' => 'password1',
'user2' => 'password2');
/* it is cruciual that you uncomment these lines if you a inserting
these values into a query to prevent sql injection. These are for
Mysql, so you my have to modify them appropriately for other
databases. */
// $username = mysql_escape_string($username);
// $password = mysql_escape_string($password);
Notice how the session is used, once logged on to store a variable to indicate the user has logged on. You should check this exists on every page that requires authentication and to log the user out you simply set this variable to false.
next_page.php contains an include to a small script called auth.php that checks for authentication. To enable authentication for a specified page, simply put this line at the top of the script:
PHP Code:
require 'auth.php';
auth.php
PHP Code:
<?php
session_start();
if (! @$_SESSION['authenticated']) {
header('Location: logon_display.php');
exit;
}
?>
I might as well continue with this thread (not sure if I should but I'll do that as the 2nd question is related)...
This is how I've been told to deal with authentication. It's quite simple I guess but it works fine for the stage I'm working at:
1 - I've made a hash field in my users table.
2 - When you register it is set to 0.
3 - When you login successfully the hash is set to a random number.
4 - I put the hash into a cookie as well.
5 - Each time a user logs in the hash is randomly generated.
6 - If I want to check if a user has logged in I just compare hash in cookie and hash field.
This is what I've got so far:
Code:
//Checks if there is a login cookie
if(isset($_COOKIE['my_hash']))
//if there is, it checks to see if my_hash = userhash
{
$c_user = $_COOKIE['my_username'];
$c_hash = $_COOKIE['my_hash'];
$check = mysql_query("SELECT * FROM Users WHERE Username = '$c_user'")or die(mysql_error());
while($info = mysql_fetch_array( $check ))
{
if ($c_hash != $info['Userhash']) // not logged in
{
}
else // logged in already
{
header("Location: users.php");
}
}
}
What I want to do is put that into a function so that I can call it whenever I need to check if the user is logged in or not. I guess because I am included username with it I can get the users details...
How should I put it into a function, and how would I correctly call it?? Thx
<?php
/* disable notices as variables will be undefined on first display */
error_reporting(E_ALL ^ E_NOTICE);
?>
Ouch, not very cool, IMO. Shutting off notices because some variables might not yet be defined works for those variables, but it also works for ALL variables, which means if you screw up somewhere on the page...you lose a quick reference to what the problem might be.
I think it'd be wiser to do something like this to take care of those variables:
I would only ever do that in a template script like the one above. I am usedto it because I use Smarty . But the best way is to check if a variable is defined before using it and define all variables that may be set.
Depends on how you are authenticating users? Are you using sessions to store an active, logged in session? If so, then you should probably check to see if the session exists, and also verify that the session data is accurate.
I'm not using sessions I'm using cookies - post #5 shows the sample code... What I want to do is put that code into something like logincheck.php so I can call that and find out if logged in or not.... Is that possible/good idea?
I getting erratic results with my login pages and as I haven't really added other security measures (yet) it's hard to confirm if I'm logged in or not, especially since I'm having to refresh every page to get it to show properly....!
Can someone please advise on a flowchart or something for doing a login check. The post above (#12) is used to check if a cookie exists but for some reason this doesn't work:
Code:
if(cookieCheck) { // logged in
header("Location: index2.php");
//print_r(cookieCheck);
} else { // not logged in
$past = time() - 100;
//this makes the time in the past to destroy the cookie
setcookie('my_username', '', $past);
setcookie('my_hash', '', $past);
header("Location: index.php");
}
What I'm doing [or trying to do] with the above code is check if the login cookies exist and if they do check their details to confirm the user is logged on ok. IF they do not exist or they do not match with whats in the db that means the user is not logged in.
What I'm getting is, even if there is nothing in the username/password form it jumps to index2.php, in other words the line
Code:
if(cookieCheck) { // logged in
header("Location: index2.php");
seems to always run!!! That must mean cookieCheck isn't working right, so what can I change?????
All I need is to put something in cookieCheck which will let me check if a user is logged in or not and then present TRUE or FALSE.... plz help!
<?php
$loggedin = false;
if(isset($_COOKE['my_username'], $_COOKIE['my_hash'])){
//check the values against the database
$user = mysql_real_escape_string($_COOKIE['my_username']);
$hash = mysql_real_escape_string($_COOKIE['my_hash']);
@extract(mysql_fetch_assoc(mysql_query("SELECT hash as authenticate FROM tablename WHERE username='$user' AND hash='$hash'")));
if(isset($authenticate)){
$loggedin = true;
//REFRESH the cookies
$renew = time() + (3600 * 24 * 3); +3 days
setcookie('my_username', $_COOKIE['my_username'], $renew);
setcookie('my_hash', $_COOKIE['my_hash'], $renew);
}else{
//invalid login info, set their cookies to expired
$expire = time() - 3600; //-60 minutes
setcookie('my_username', '--', $expire);
setcookie('my_hash', '--', $expire);
}
}
if($loggedin)
header("Location: ./index2.php");
else
header("Location: ./index.php");
//you don't need to set the cookies to not exist if they aren't logged in. this will somewhat save on the amount of queries sent to the database
just a side note: you should check if the user is logged on with an include on every page (or at least, that's what I usually do). This way, someone couldn't just type index2.php in their browser's address bar and be "logged in."
Wow excellent! Well actually this is what I was doing, can you let me know how the above would fit in with my psuedocode:
Code:
1. Get hash info;
2. If (COOKIEHASH <> DBHASH) // not logged in
{ *make session var with page url
*go to login page (which will check if session var exists, if it does it will check login and then go to page url in session var)
}
3. Get Levelid // each user has a levelid
if (Levelid <> Page levelid) // user does not have access to this page
{ die ("You do not have access to this page");
So that's what I was thinking, I duno if it is the best way but it seems right for me at this stage...
I would run this on every page which needs the user to be logged in. However I understand that if I have it on every page I might as well put it into a function! How can I combine it with what you have above to do just that as that will save me loads of time and space
well, you don't have to make a function.. you can just have an include that does it. it's a little pointless, to me, to make it a function, seeing as how you will have to include the "function" script into every page anyway; and then you'd also need to call the funciton as well. it's counter-intuitive.
for your include, though, you could basically do this:
authenticate.inc.php:
PHP Code:
<?php
$loggedin = false;
if(isset($_COOKIE['my_username'], $_COOKIE['my_hash'])){
$user = mysql_real_escape_string($_COOKIE['my_username']);
$hash = mysql_real_escape_string($_COOKIE['my_hash']);
@extract(mysql_fetch_assoc(mysql_query("SELECT level as authenticate FROM table_users WHERE hash='$hash' AND username='$user' LIMIT 1")));
if(isset($authenticate)){
//global usage variables
$loggedin = true;
$login = array();
$login['user'] = $_COOKIE['my_username'];
$login['level'] = $authenticate; //this sets their level
//refresh their cookies
$renew = time() + (3600 * 24 * 3); //+3 days
setcookie('my_username', $_COOKIE['my_username'], $renew);
setcookie('my_hash', $_COOKIE['my_hash'], $renew);
}else{
//bad login, expire their cookies
$expire = time() - 3600; //-60 minutes
setcookie('my_username', '--', $expire);
setcookie('my_hash', '--', $expire);
}
}
//if they aren't logged in, let's redirect them
if(!$loggedin)
header("Location: ./login.php");
//if they ARE logged in, everything else included AFTER this page will show up fine
?>
sample use:
index.php:
PHP Code:
<?php
require_once("mysql.inc.php"); //mysql database information/login
require_once("authenticate.inc.php"); //authentication script above
//set this page's values (can easily set all of this stuff with a database query, too)
$this['level'] = 5;
if($login['level'] < $this['level'])
die("You aren't authorized to view this page!");
?>
<html>
<head>
<title>Super Secret Location</title>
</head>
<body>
<h1>hello, <?php echo $login['user']; ?>!</h1>
</body>
</html>
Well, a while back I wrote a very secure login script that allows basically everything you need. I outsourced it to many people on my web host's forums and they used it quite often and gave it some great reviews.
Take a look at it, it is completely open-source, I do not require credit as long as you are not taking credit for it.
The source is EXTREMELY simple and includes some pretty nice functions for redirecting without headers. I would check it out, and tell me if you like it or not. I have included it as an attachment. But the code is also below:
mysql_connect($server,$username,$password);
@mysql_select_db($database) or die("Unable to select database");
$query="SELECT * FROM users";
$result=mysql_query($query);
$num=mysql_numrows($result);
//Checks If The Username and Password Exists
//Declaring Variables
$usernameisvalid=0;
$passwordisvalid=0;
$passwordexists=0;
$usernameexists=0;
//loop through all the entrys in the database to see if any match...
while ($usernameisvalid < $num)
{
//set the username they want to a variable
$checkuser=$username1;
//Set the database user name into a variable
if ($passwordexists == 0 or $usernameexists == 0)
{
echo "This Username or Password is Invalid!";
}
if ($usernameexists == 1 and $passwordexists == 1)
{
echo "Loading Members Page...";
$_SESSION['LoggedIn'] = True;
$_SESSION['UserName']= mysql_result($result,$DBspotnew,"Username");
$_SESSION['PassWord']= mysql_result($result,$DBspotnew,"Password");
mysql_close();
js_redirect_loggedin();
}
}
if($action == "loggedin")
{
if(isset($_SESSION['LoggedIn']))
{ //Checks if the session named "Logged In" is set to 1
include('memberspage.php');
}
else
{
echo "You are not logged in!";
}
}
If you did not notice, you have to change some things, take a look through the code and do not use it directly or it will not work, you have to suit it to your likings.
I hope the following doesn't come off to you as an attack, Seraphino; I am just curious and am very confused as to why you did your login script the way you did.
first of all: it is VERY, VERY, VERY inefficient for you to loop through EVERY record in a table just to check if some user exists. the overhead from PHP alone will add tons of executing time to your script depending on how many users you have; and what if you had a million? you should be using your WHERE clause to select only the record you want (the record that has a username equal to $_POST['username'] and also has a password equal to $_POST['password']), and let MySQL do ALL of the work for you. doing it in PHP is not more secure, or not more anything. It's more work that you don't need to do. Make sure you validate the user input, too (eg: functions like mysql_real_escape_string()), so that you can protect yourself from SQL injection.
how is what you posted "very secure" if memberspage.php doesn't even check for sessions or anything? someone could just type that address into their browser and view it.
also, using PHP's headers is a much better idea if you're doing it before any output has been given to the browser; not all users have javascript enabled. some old browsers don't even support the javascript redirect you used. it's much, much easier to use a built-in PHP function than to make two functions that just output javascript. if you're making an administration module, you don't want anyone who shouldn't have the ability to potentially view anything, so using headers for redirection is great.
a few more questions/comments:
what's the point in having your stylesheet at all in memberspage.php if you don't use the #layout entity?
you shouldn't be including a page that has all the making of an HTML page into a page that already has its own DOM (eg: your index.php always outputs <html> and <body> tags, and it's incorrect for you to be including a new file with its own <html> and <body> tags (memberspage.php))
did you not post the full source of these pages..? or am I missing something?
I don't consider it an attack. What I was mainly talking about with security is the login itself. The members page was barely worked on by myself and I count on many people to replace it with what they want because it is really nothing.
Also, looping through the database takes under 2 seconds even with over 500 users in the database, so it is not like it is going to take an hour to do something like that.
Of course, I always planned on redoing the script anyway because there are many things I would want to upgrade on the script.
Well, a while back I wrote a very secure login script that allows basically everything you need. I outsourced it to many people on my web host's forums and they used it quite often and gave it some great reviews.
As Mr T would say: "I pity the fools".
Originally Posted by Seraphino
Also, looping through the database takes under 2 seconds even with over 500 users in the database
Try with 50,000+ users. Or a slow connection to the database. Returning the resultset alone will kill it.
Also, doing anything that can be done using headers, by other means, is bad.
Originally Posted by Seraphino
Of course, I always planned on redoing the script anyway because there are many things I would want to upgrade on the script.
Why post it then?
Originally Posted by kows
Make sure you validate the user input, too (eg: functions like mysql_real_escape_string()), so that you can protect yourself from SQL injection.
Use a proper data access library that supports parameterised queries, then you never have to mess around escaping anything.
Also, with regards to the code kows posted, remember in a production environment to always validate the results of functions such as mysql_query and mysql_fetch_assoc, and not to use die() error messages.
I love how you don't even have to login to access the members page
Originally Posted by Seraphino
I outsourced it to many people on my web host's forums and they used it quite often and gave it some great reviews.
Maybe they should learn PHP.
Originally Posted by Seraphino
The source is EXTREMELY simple
That has got to be the most complex login script I have EVER seen.
Originally Posted by Seraphino
and includes some pretty nice functions for redirecting without headers.
Redirecting using Javascript is in no way favourable over using the builtin headers provided by HTTP and browsers with Javascript disabled will not work at all.
Originally Posted by Seraphino
If you did not notice, you have to change some things.
Thanks loads guys!!! In the end this is what I've gone with, I'm sure its not the most advanced solution but I guess it does the job:
On each page which needs authentication I've added the following lines:
Code:
ob_start();
at the top
Code:
require_once("auth.php");
// check levelid is ok for this page
if ($db_level<>1) // user doesnt have access
{
die("You don't have access to this page");
}
in the middle, the check level is different for different pages of course
Also, looping through the database takes under 2 seconds even with over 500 users in the database, so it is not like it is going to take an hour to do something like that.
You need more than one second to find one particular record among 500? That is, of course, if only one user is accessing the database at any time.
And no alarm bells are ringing? Hell, they should be falling off the bell tower!
For reference, doing this the proper way typically takes a database server around a millisecond on your typical "I converted an old computer" development server. Your script is three orders of magnitude slower.
All the buzzt CornedBee
"Writing specifications is like writing a novel. Writing code is like writing poetry."
- Anonymous, published by Raymond Chen
Don't PM me with your problems, I scan most of the forums daily. If you do PM me, I will not answer your question.
Yeah, I know this is a flame war against me, but if you didn't notice, this was one of the first scripts that I ever wrote from memory of PHP (no book or anything).
Plus, I believe I mentioned that I planned on re-writing it someone to take into account all the skills I have acquired since I first wrote that script.
but if you're still handing out that script and trying to defend it, you can't be trying to say you've acquired a lot of skills since you first wrote that script.. if you did, you would know how wrong it was.
Starting off with login/registration
Reply With Quote
Originally Posted by visualAd
. But the best way is to check if a variable is defined before using it and define all variables that may be set.
CornedBee