PHP Login/User Creation Validation Function

I'm VERY new to PHP and have begun writing a basic login for a website that uses data from a Microsoft SQL server to display information on the web. Below are 3 snippets of code (two functions from the same file and one controller file that calls them.)

Connect to MSSQL to validate user (from phplib)

With this, I take the login credentials stored in a MySQL database, and use them to connect to a MSSQL database. (As seen in the "createUser" case of the controller).

```
function PDOConn(&$IPAddress, &$userAdmin, &$passAdmin, &$Database, $sess_client)
{
global $host,
$username,
$password,
$db_name,
$tbl_name_client;

try {

$conn = new PDO("mysql:host=$host; dbname=$db_name", $username, $password);
$stmt = $conn -> prepare("SELECT stuff
FROM $tbl_name_client
WHERE client = :clientId");
$conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$stmt->execute(array('clientId' => $sess_client));
$result = $stmt->fetchAll();

if ( count($result) == 1 )
{
foreach($result as $row)
{
$IPAddress = $row['IP'];
$userAdmin = $row['Username'];
$passAdmin = $row['Password'];
$Database = $row['database'];
}
}
else if (count($result) > 1)//Primative error handling
{
exit( "There are multiple rows in the clients table with the same clientID, which is really strange...");
}
else
{
exit( "ID isn't in the table. No rows returned. Sorry bro...");
}

First off, welcome to Code Review. Answers here may be a while in coming. Usually I'm better about getting to these questions but I've been extremely busy the last week and so missed this. Hopefully you're still watching this.

Learning MySQL is a great way to get comfortable with the idea of SQL, but PDO or MySQLi are the ways to go. MySQL is soon to be deprecated. So heads up. To the best of my knowledge, PDO is not expiramental, but then again, my knowledge is limited when it comes to SQL. That's not to say that my answer won't be full of good information, only that I can't help you with specifics in this regard. I've helped people with PDO and MySQLi code on here before without needing to know how it works. I do normally suggest they seek further opinions, but that's a general disclaimer about anything seen on the internet.

Anyways, without further ado...

Globals and Security

Are bad. If you want a piece of information to be available across scripts, then either pass it through POST, GET, SESSION, or COOKIE. Which depends on the type of information being passed. For instance, anything private should only be passed via post, and is usually done via customer to server and you will have no control over it. Anything public can be passed with get, and saved with cookies. Anything session specific to a customer should be passed with sessions.

So your comment about being ahead of the game compared to LinkedIn was wrong. Your passwords are freely available on the default global environment, unsalted and unprotected. Besides, I think the specific instance with LinkedIn was not that they weren't salting their passwords, it was that they were using SHA to do it. Though I may be misremembering my facts here. I am by no means a security expect, so, if you'll pardon the pun, take my advice with a grain of salt.

Functions and OOP

Functions should be specialized. In other words they should do only one thing, and do it well. So PDOConn() should only be worried about connecting to the database. A separate function should be concerned with preparing and executing statements. This is the first step to learning OOP, a subject that baffles many. Once you are familiar with this concept, and the DRY principle, then you might be ready for OOP.

Naming conventions say that only the first letter in every word should be capitalized and only on interior words. At least when using camelcase. So your function should actually be pdoConn(). Only classes should capitalize the first letter in the first word, and even then it would only be PdoConn(). Think of it like this. A class is an object, or noun, a function is an action, or verb. You capitalize nouns, you don't capitalize verbs.

Exiting a Script

Exit is used to tell PHP to cancel a script. If you want to set some output, you should do so before calling exit, usually with echo, though some people prefer print. While the way you are doing it is technically OK, it is usually better to separate the tasks.

Storing and Retrieving Passwords

I can't imagine a reason to have a password stored on every row of a table unless that table was the user credentials table. In which case you should not be traversing the entire table to get a single login, you should instead retrieve the row specific to your user and compare their information to that single row. Besides, on the off chance that two customers share a password, you will be faced with the real possibility of a user getting the wrong account credentials.

Breaking the Loop

When you have found what you were looking for in a loop, you should break from it to prevent further iterations. This saves processing power. Now, before you say, "But I did, look at that header()!", know that you did not. In order to properly terminate a script after calling a header with the location flag, you must also call the exit() function immediately afterwards, else the script will continue to run. Additionally, when using a header, it is proper to use an absolute path instead of the relative one.

Output Buffering

Asides from you calling an include before the session_start() I see no reason for this. You are not processing the information buffered, you are simply dumping it. I'd remove this buffer and just move the include to below the session.

Another Security Concern

You are using direct user input without validating or sanitizing it. This is a concern for XSS. If you have PHP version >= 5.2 you can use PHP's filter_input() function, for example:

$email = filter_input( INPUT_POST, 'email', FILTER_SANITIZE_EMAIL );

Controllers

I'm assuming you are talking about the OOP approach MVC here. A controller, normally, only focuses on one view (case). However, I can see that using the same controller for similar views, such as adding/removing a user, is understandable and I don't really see anything wrong with it. But I would limit that as much as possible. If the controllers start getting too large or difficult to follow, then

StackExchange Code Review Q#13639, answer score: 2