YankeeFanInKC Posted November 11, 2010 Share Posted November 11, 2010 I'm developing my own CMS with a few functions and wanted to know how things are looking right now because I can't find a board for strickly CODING CRITIQUE so I put it in this board. There isn't a whole lot to go through. I know there is something wrong with my issets line but other than that just a general critique of how its shaping up? manager.php <?php session_start(); require "dbconfig.php"; require "functions.php"; if ((isset($_POST['username'])) && (isset($_POST['password']))) { $username = $_POST{'username'}; $password = SHA1($_POST{'password'}); validate($username, $password); } elseif ((!(isset('username'))) && (!(isset('password')))) { require_once "login.php"; } $username = stripslashes($username); $password = stripslashes($password); $username = mysql_real_escape_string($username); $password = mysql_real_escape_string($password); $sql="SELECT * FROM dbusers WHERE username='$username' and password='$password'"; $result = mysql_query($sql); ?> functions.php <?php // This page defines functions used by the login/logout process function validate($username, $password) { } ?> login.php <?php include_once ("globals.php"); ?> <html> <head> <title><?php echo $shortsitename; ?> EW Manager</title> <link rel="stylesheet" type="text/css" href="<?php echo "$stylesheet"; ?>" /> </head> <body> <p id="backtosite"><a href="#" title="Are you lost?">← Back to <?php echo $fedname ?></a></p> <div id="login"> <h1><?php echo $shortsitename; ?> Manager</h1> <form id="loginform" action="" method="POST"> <p><label>Username<br /><input type="text" name="username" id="user_login" class="input" size="15" /></label></p> <p><label>Password<br /><input type="password" name="password" id="user_pass" class="input" size="15" /></label></p> <p class="forgetmenot"><label><input name="rememberme" type="checkbox" id="rememberme" /> Remember Me</label></p> <p class="submit"> <input type="submit" value="Login" class="button-primary" /> </p> </form> </div> </body></html> Quote Link to comment Share on other sites More sharing options...
fonor Posted November 12, 2010 Share Posted November 12, 2010 Hy my friend, Your erro is in this line: elseif ((!(isset('username'))) && (!(isset('password')))) use $_POST on this isset TO try: elseif ((!(isset($_POST['username']))) && (!(isset($_POST['password'])))) If you have any questions about isset try this site: http://www.ogenial.com.br/blog/isset-verifica-se-uma-variavel-existe/ Quote Link to comment Share on other sites More sharing options...
coupe-r Posted November 12, 2010 Share Posted November 12, 2010 So far it looks OK, but you do not need: elseif ((!(isset('username'))) && (!(isset('password')))) just use: elseif (!isset('username') && !isset('password')) For your password SHA1, you may want to look a recent post in this forum to SALT your password. Quote Link to comment Share on other sites More sharing options...
Pikachu2000 Posted November 12, 2010 Share Posted November 12, 2010 I'd make a few changes. See the comments in the code. You could also make the form fields "sticky" so the user can correct them if an error is made. <?php session_start(); require "dbconfig.php"; require "functions.php"; if ( isset($_POST['username']) && isset($_POST['password']) ) { // consider checking that trim()med values are !empty() instead of using isset() $username = $_POST['username']; // square brackets rather than curly braces are standard for enclosing array indices $password = SHA1($_POST['password']); validate($username, $password); // this doesn't actually do anything . . . //$username = stripslashes($username); Unless magic_quotes_gpc is on, you shouldn't be using stripslashes() on form data //$password = stripslashes($password); $username = mysql_real_escape_string($username); // $password = mysql_real_escape_string($password); There is no need to use mysql_real_escape_string on a hashed value $sql="SELECT * FROM dbusers WHERE username='$username' and password='$password'"; $result = mysql_query($sql); } else { require_once "login.php"; } ?> Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.