Why is Jason schreefing again?
Jochem Maas wrote:
> Jason Pruim schreef:
>>
>> On Mar 18, 2008, at 3:20 PM, Jochem Maas wrote:
>>
>>> what started out as a simple little reply bloated out into an
>>> inpromptu brain
>>> fart ... lots of bla .. enjoy :-)
>>>
>>> Jason Pruim schreef:
>>>> Hi everyone,
>>>> I am attempting to add a little error checking for a very simple
>>>> login system. The info is stored in a MySQL database, and I am using
>>>> mysqli to connect to it. I have it working with the solution
>>>> provided below, but I am wondering if this is the right way to do it
>>>> or if there is a better way?
>>>
>>> at an abstract level you might consider that your function could simply
>>> always return a boolean (true = logged in, false = not logged in) and
>>> that the
>>> rest of the application retrieves all the other data via the session
>>> (as opposed to returning half the data and storing half in the session)
>>
>> I think this is what I am attempting to do... Just going about it all
>> wrong...
>
> start from scratch again?
>
>>
>> I want the pages to check to see if the person is still logged in and
>> if they are, then it's pulling live data from the database... So
>> maybe I should edit my authentication function...
>
> maybe.
> there are two different things being confused:
>
> 1. checking logged in state.
> 2. attempting to login.
>
> function getUserData()
> {
> if (isAuthenticatedUser())
> return $_SESSION['user']['data'];
>
> return null;
> }
>
> function isAuthenticatedUser()
> {
> return (isset($_SESSION['user']['authenticated']) &&
> $_SESSION['user']['authenticated']);
> }
>
> function authenticateUser($u, $p, $cc = false)
> {
> if (($iau = isAuthenticatedUser()) && !$cc)
> throw Exception('Already logged in!');
>
> $cmd = $iau ? 'verify account' : 'login';
>
> if (!($p = trim($p)) || !($u = trim($u)))
> throw Exception('Cannot '.$cmd.' without credentials!');
>
> $p = mysql_real_escape_string($p);
> $u = mysql_real_escape_string($u);
>
> if (!($res = mysql_query("SELECT * FROM `users` WHERE 'pwd'='$p' AND
> `usr`='$u'")))
> throw Exception('Cannot '.$cmd.', verification system error.');
>
> if (mysql_num_rows($res) != 1)
> return false;
>
> if (!($row = mysql_fetch_assoc($res)))
> throw Exception('Cannot '.$cmd.', verification system error.');
>
> if ($iau)
> return (int)$_SESSION['user']['data']['id'] === (int)$row['id'];
>
> unset($row['pwd']);
>
> $_SESSION['user'] = array(
> 'authenticated' => true,
> 'data' => $row,
> );
>
> return true;
> }
>
>>
>> function auth($loggedin) {
>> query database to see if username & Password match;
>> write certain variables into session (Or maybe into the cache?)
>
>
>
>> return true if it matches
>> if not return false which could then redirect back to login page...
>> }
>>
>> Is it that simple? Am I trying to make things so much more complicated?
>>>
>>>
>>> if you choose to store everything and only return authentication
>>> state you
>>> might also consider to abstract the storage somewhat so that other
>>> code doesn't
>>> have to access the session data directly. we call this concept 'loose
>>> coupling'.
>>> for instance:
>>>
>>> function getUserInfo($key = null)
>>> {
>>> if (!isset($_SESSION['user']['loggedin']))
>>> return null;
>>>
>>> if (!$_SESSION['loggedin'])
>>> return null;
>>>
>>> $key = trim((string)$key);
>>>
>>> if ($key == '')
>>> return $_SESSION['user'];
>>>
>>> if (isset($_SESSION['user'][$key]))
>>> return $_SESSION['user'][$key];
>>> return null;
>>> }
>>>
>>> this example still requires that the the consumer of getUserInfo() knows
>>> the names of the relevant columns (from multiple tables?)
>>
>> login info is stored on 1 table, while the actual records in the DB
>> are stored on another table. After successful login it changes from
>> the login table to the data table.
>>
>>> .. this could also
>>> be abstracted, a simple solution would be something like:
>>>
>>> // put these in a config file, (CKEY = 'cache key' ... just a thought)
>>> define('CKEY_USER_NAME', 'loginName');
>>> define('CKEY_USER_LEVEL', 'adminLevel');
>>> define('CKEY_USER_TABLE', 'tableName');
>>>
>>>
>>> $uName = getUserInfo( CKEY_USER_NAME );
>>> $uLevel = getUserInfo( CKEY_USER_LEVEL );
>>> $uLevel = getUserInfo( CKEY_USER_TABLE );
>>
>> And then that would hold the info in a cache until the user hit logout
>> and then logged back in? I'm going to try that right after sending
>> this message.... That may work perfectly...
>>
>> Also I'm assuming if I put these into an include file it will work
>> just like my other variables where I can call $pass from any page that
>> includes the file $pass is defined in?
>>>
>>>
>>> ... you get? ... incidentally your column names seem to be
>>> case-sensitive,
>>> I recommend lower or upper (depending on DBMS) case only for sql
>>> entity names
>>> for two reasons:
>>>
>>> 1. you avoid nitpicky irritations due to SQL case-sensitivity related
>>> bugs
>>> in your code.
>>>
>>> 2. if you lowercase all entity names you can write stuff like so:
>>>
>>> $sql = "SELECT foo, bar FROM qux WHERE abc = 1 AND def=2";
>>>
>>> which is a little more readable than this:
>>>
>>> $sql = "SELECT FOO, BAR FROM QUX WHERE ABC = 1 AND DEF=2";
>>>
>>> of course it should be more like:
>>>
>>> $sql = "SELECT `foo`, `bar` FROM `qux` WHERE `abc`=1 AND `def`=2";
>>>
>>> using case to differentiate between SQL and entity names becomes more
>>> useful
>>> as the queries become more complex. I also tend to then break then up
>>> into lines:
>>>
>>> $sql = "SELECT
>>> q.`foo', q.`bar`,
>>> na.`foo` AS nafoo, na.`bar` AS nabar,
>>> noo.`foo` AS noofoo, noo.`bar` AS noobar,
>>> FROM
>>> `qux` AS q
>>> LEFT JOIN
>>> `na` AS na ON na.`qux_id` = q.`id`
>>> LEFT JOIN
>>> `noo` AS noo ON noo.`qux_id` = q.`id`
>>> WHERE
>>> (`abc`=? AND `def`=?)
>>> AND
>>> q.`id` IN (SELECT `qux_id` FROM `quxnobbins` WHERE
>>> `nobbin_id`=?)
>>> AND (
>>> (`start_date` BETWEEN ? AND ?) OR
>>> (`start_date` BETWEEN ? AND ?)
>>> )";
>>>
>>>
>>>
>>>> My thinking with this is if more then 1 record is returned from the
>>>> database, then there is a issue... If only is returned then the
>>>> username/password matched and I can safely show them the info...
>>>> $rowcnt = mysqli_num_rows($loginResult);
>>>
>>> we'll assume the original sql was suitably prepared (i.e. user values
>>> escaped, etc).
>>> but why not 'fix' the query and/or table so that it will only ever
>>> return one row?
>>>
>>>> if($rowcnt !="1"){
>>>
>>> avoid auto-casting!
>>>
>>> if ($rowcnt !== 1) { /*...*/ }
>>>
>>>> echo "Auth failed";
>>>> die("Auth failed... Sorry");
>>>> }else{
>>>> while($row1 = mysqli_fetch_array($loginResult)) {
>>>
>>> this 'while' is completely pointless, you know there is just one row,
>>> no point in looping for a single iteration.
>>
>> Will make that change now :)
>>>
>>>
>>> just do:
>>>
>>> $row = mysqli_fetch_array($loginResult);
>>> $_SESSION['user'] = $row['loginName'];
>>> // ... etc
>>>
>>>
>>>> $_SESSION['user'] = $row1['loginName'];
>>>> $_SESSION['loggedin'] = "YES";
>>>
>>> "YES" is not a boolean value, I think $_SESSION['loggedin'] should be
>>> boolean (you got deja vu here also?).
>>
>> Just to double check: $_SESSION['loggedin'] = TRUE; //Is a boolean while:
>> $_SESSION['loggedin'] = "TRUE"; // is not correct?
>>
>>>
>>>
>>> check the following code to see why:
>>>
>>> $_SESSION['loggedin'] = "FALSE";
>>> if ($_SESSION['loggedin'])
>>> echo "your logged in!";
>>>
>>>
>>>
>>>> $table = $row1['tableName'];
>>>> $adminLevel = $row1['adminLevel'];
>>>> $authenticated = "TRUE";
>>>
>>> again the boolean should be boolean!
>>>
>>>> echo "<BR>authentication complete";
>>>
>>> with regard to seperation of responsibilities: the function should
>>> really be either attempting an authentication *or* outputting some
>>> message
>>> regarding the result of the authentication attempt but *not* both.
>>
>> That was added for debugging, helping me track down where the error was.
>>>
>>>
>>> in practice this means my recommendation would be to remove the echo
>>> statements
>>> from the function and have the code that calls this function be
>>> responsible for
>>> outputting feedback ... imagine if you need to, someday, perform an
>>> authentication
>>> without [direct] output? or you need to change the outputted message
>>> under certain
>>> conditions (conditions which are outside the scope of this function)?
>>>
>>> a function should, as much as is possible, do one thing only (and do
>>> it well), otherwise,
>>> I guess, it would be called a functions. ;-)
>>>
>>>> }
>>>> return Array($table, $authenticated, $adminLevel);
>>>
>>> pretty much the rest of the world writes 'Array()' as 'array()' ..
>>> the convention
>>> being that built in functions and lang constructs are always typed
>>> lowercase. some
>>> people write things like isSet($foo); ... but they are 'wrong' :-)
>>
>> I thought I saw on the php.net page that it was Array() :)
>>>
>>>
>>> I generally try to distinguish between userland and php functions by
>>> using lowercase
>>> for php funcs and CamelCase naming schemes for userland functions.
>>
>> I see what you're getting at though... And I need to do that more
>> through my applications..
>>
>>
>>>
>>>
>>> --
>>> "ok, porky pig say your line."
>>>
>>
>> --
>>
>> Jason Pruim
>> Raoset Inc.
>> Technology Manager
>> MQC Specialist
>> 3251 132nd ave
>> Holland, MI, 49424-9337
>> www.raoset.com
>> japruim@xxxxxxxxxx
>>
>>
>>
>
--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
