Page 2 of 3

Posted: Wed Aug 02, 2006 7:05 am
by Ambush Commander
Okay, I'm seeing an interesting pattern here: It seems that if a user is authenticated, that means that they are also authorized. Good for small systems that only need logins for administration, but you may want to make the distinction clear if you want to roll login out to the entire system.
what can I do to sum all of this up into a class or two that can be easily adapted or extended to allow for one-time token and/or remember me?
You'll actually need a few classes. First, factor out all the functionality associated with verify(): you can call this the "Login" process.

Posted: Wed Aug 02, 2006 7:18 am
by jayshields
Well, on a side note, I don't care for uber security on this little app I'm making at the moment, so this is how I do authentication on the admin panel:

index.php

Code: Select all

<?php

include('config.php');
include('login.php');

echo 'you\'re logged in!';

?>
config.php

Code: Select all

<?php

define('USER', 'yourusername');
define('PASS', 'yourpassword');

?>
login.php

Code: Select all

<?php

session_start();

if(isset($_POST['username']) && isset($_POST['password'])) {
	if($_POST['username'] == USER && $_POST['password'] == PASS) {
		$_SESSION['logged'] = TRUE;
	} else {
		$message = '<font color="red">The username/password entered was incorrect.</font>';
	}
}

if(!isset($_SESSION['logged'])) {
	echo '
	<html>
		<head>
			<title>Login</title>
			<LINK REL=stylesheet HREF="includes/style.css" TYPE="text/css" />
		</head>
		
		<body onLoad="document.loginform.username.focus()">
			<center>
				<font face="Verdana">';
					if (isset($message)) { //If the error was set, show it
						echo $message, "<br>";
					}
					echo '
					<form name="loginform" action="index.php" method="post">
						<table border="0">
							<tr>
								<td>Username: </td>
								<td><input class="text" type="text" name="username" size="20" maxlength="20" /></td>
							</tr>
							<tr>
								<td>Password: </td>
								<td><input class="text" type="password" name="password" size="20" maxlength="20" /></td>
							</tr>
						</table>
						<input class="submit" type="submit" name="login" value="Login" />
					</form>
				</font>
			</center>
		</body>
	</html>';

	die();
}

?>
By this discussion, I'm starting to think this is probably very insecure in some way?! :D

Posted: Wed Aug 02, 2006 7:33 am
by thiscatis
is there a reason why you don't store your password in a database?
You also don't have a anti-hacking attempt in your config.
Or i'm missing something completely here :D

Posted: Wed Aug 02, 2006 7:49 am
by jayshields
thiscatis wrote:is there a reason why you don't store your password in a database?
You also don't have a anti-hacking attempt in your config.
Or i'm missing something completely here :D
I don't need to store user details in a db, it's only a small app, the user will never need to change user/pass and if more people want to use the system they can just tell them the pass.

Anti-hacking attempt? The only thing I'm thinking you mean is something to stop someone externally including my config.php and echo'ing the constants, but that's a bit farfetched anyway since they'd have to guess my directory structure, filename, and constant names (not like that would be too difficult, I haven't gone out of my way to make it hard, lol).

Posted: Wed Aug 02, 2006 10:17 am
by Luke
Jay, Security through obscurity is definately not security.

Posted: Wed Aug 02, 2006 10:32 am
by jayshields
Yeah, I was going to say I didn't agree with that in my post but I never really thought about my authentication that way before. I'll change it now to incorporate some sort of check.

Posted: Wed Aug 02, 2006 11:07 am
by Christopher
The Ninja Space Goat wrote:Jay, Security through obscurity is definately not security.
Well yes and no ... you understand that in password based systems the password itself is Security Through Obscurity. It is a common component of security systems. The problem is when it is the only mechanism.

In discussions on this subject I have talked about having the authentication system just manage the middle part and push off the specific stuff to the application. I know Ambush and I disagree on this -- he takes a more full-featured, bottom up approach (in my opinion).

I have been thinking of something more like this -- obviously simplified:

Code: Select all

class Authenticate {
        private $authorizor;
       
        public function __construct(Authorizor $authorisor){
                $this->authorizor = $authorisor;
        }

        public function isAuthorized(Credential $credential){
               
                 return $this->authorizor->check($credential);
        }

}
So it becomes a case of creating Credential/Authorizor pairs for each type of authentication you want to support.

Posted: Wed Aug 02, 2006 11:28 am
by Luke
I like that idea arborint... I don't know exactly how I would make the authorizor and credential classes, but i like where you are going. I will mess around with that idea. thanks

Posted: Wed Aug 02, 2006 11:47 am
by Christopher
For a username/password system it could be as simple as:

Code: Select all

class Credential_UsernamePassword {
        public $username;
        public $password;
}

Code: Select all

class Authenticator_UsernamePassword {
        private $db;

        function __construct($db) {
                $this->db = $db;
        }

        function check(Credential_UsernamePassword $credential) {
                $query = 'SELECT * FROM `users` WHERE `username` = "' . $credential->username . '"';
                $user = $this->db->query($query);
                return ($credential->username == $user->username) && ($credential->password == $user->password);
        }
}
EDIT - changed Authorizor to Authenticator

Posted: Wed Aug 02, 2006 11:51 am
by Luke
NICE! So simple. This is exactly what I was looking for! Thank you chris... you are the man!

Posted: Wed Aug 02, 2006 3:26 pm
by Luke
i like this much better

Code: Select all

$Registry->register('AuthorizorUsernamePassword', null, $Registry);
	$authorizor = $Registry->get('AuthorizorUsernamePassword');
	
	$credentials = $Registry->get('CredentialUsernamePassword');
	
	$credentials->username = $request->get('username');
	$credentials->password = $request->get('password');
	
	$Registry->register('Authenticate', null, $authorizor);
	$authenticate = $Registry->get('Authenticate');
	
	$auth = $authenticate->isAuthorized($credentials);
		
	if(!$auth) $Template->assign('message', 'Invalid login: Please try again...');

Posted: Wed Aug 02, 2006 3:48 pm
by Christopher
I like that style too now that I see it. I have been thinking about it since the Authentication thread but have not had time to focus on it. Perhaps I will make an attempt at a generalized version with some code for different things like N factor login, remember me and captcha to see how the code looks.

Posted: Wed Aug 02, 2006 3:54 pm
by Luke
if you did, I would surely borrow from it. :D

Posted: Wed Aug 02, 2006 4:55 pm
by Ambush Commander
I know Ambush and I disagree on this -- he takes a more full-featured, bottom up approach (in my opinion).
I suppose you could call it bottom up.

Since it seems that Ninja Space Goat is going with Arborint's design, I'll try to comment on how this design would be extended.

The first trouble is establishing the session, since as it stands right now, there's no facility for making sure that the user doesn't have to log in every page request. I would say that the return value isAuthorized(), but we'll also need a user id. There's two ways to go about doing this:

1. Simple (and hacky): Have isAuthorized() return the user ID: it still works as a boolean but also tells us who the user is.
2. Complicated (but extensible): Create an AuthorizationStatus object that holds the user ID, as well as anything else that could possibly be useful for authorization (like whether or not the user logged in via a real login or remember me).

Let's add a Captcha now. Say instead of a regular User Password login, now we need a Captcha to be verified too (we'll assume that some other part of the application automatically changes the authorizer). While we could extend the Authorizor_UsernamePassword class, I think creating an Authorizor_Composite and Authorizor_Captcha would be far more reusable. The composite authorizor would require that all the authorizors it consists of pass for it to say "authorized!" while the Captcha authorizor would solely check whether or not the Captcha works. We'd also need a CompositeCredential class.

Hope I didn't lose anyone.

Posted: Wed Aug 02, 2006 5:44 pm
by Luke
ambush: That will take me a moment to fully process... for now, I have a question...

For all subsequent pages (after login)... After user & password are verified, I store a hash of user id and username in the session and check for that on each additional page with this...

Code: Select all

class AuthorizorKey {
        private
        $db,
        $doHash,
        $hashAlg;

        public function __construct(Registry $registry, $hash=true) {
                $this->db = $registry->get("MySQL");
                $this->doHash = $hash;
                $this->setHash();
        }

        public function check(CredentialKey $credential) {
                $query = 'SELECT user_id, username FROM `users` WHERE `username` = "' . $credential->username . '"';
                $user = $this->db->query($query);
                return $user->length() ? ($credential->key == $this->hash($user->username . $user->user_id)) : false;
        }
        
        private function hash($value){
	        $alg = $this->hashAlg;
	        return $this->doHash ? $alg($value) : $value;
        }
        
        public function setHash($alg=false){
	        $this->hashAlg = $alg && in_array($alg, hash_algos()) ? $alg : 'sha1';
        }
}
Is this improving security, or have I missed the point?