Page 1 of 1

Naming advice... user class

Posted: Mon Oct 16, 2006 10:38 pm
by Luke
Alright... I've got a class called User, which is used to add/edit users in the database. Now I am going to extend this class and use it to check privileges, get username, pretty much anything having to do with the user who is viewing the site. What should I call this class?? I can't come up with anything but CurrentUser, which I don't really like.

Code: Select all

<?php
class CurrentUser extends User{

	protected $loggedIn = false;
	
	protected $session;
	
	public function __construct(){
	
		parent::__construct();
		$this->session = new MC2_Session_Mysql('CurrentUser', null, self::$_mysql->getConnection());
		if($this->session->has('user')) $this->loadId($this->session->get('user'));
	
	}

	public function LogIn($username, $password){
	
		$this->load("`username` = '" . $this->escape($username) . "'");
		$password = sha1($password);
		if(($this->username == $username) && ($this->password == $password)){
			$this->session->register('user', $this->getRaw('id'));
			// Write whatever's left in the session to disk and regenerate id
			$this->session->writeClose();
			$this->session->regenerateId();
			return $this->loggedIn = true;
		}
		// Username and password not found, reset and return false
		$this->reset();
		return false;
	
	}

	public function forceLogin(){
	
		$this->loggedIn = true;
	
	}

	public function isLoggedIn(){
	
		return $this->loggedIn;
	
	}

        public function hasPrivilege($key){

                return isset($this->privileges[$key]) ? $this->privileges[$key] : false;

        }

}
?>

Posted: Mon Oct 16, 2006 11:07 pm
by alex.barylski
Hmmm...it's looking or sounding alot like something of an authentication/authorization layer...

However, I can't help but feel something of the 'chicken and egg' syndrome...as I would argue that a user conceptually is layered on top of authentication/authorization/etc...

Layered might be the wrong word here, as I don't think extension is the ideal way to go...

Authentication/Authorization are different processes...so I'm not sure bundling them into a single class is the best solution.

What do you think? Do you have any reasons as to keep them togather except for the sake of brevity or simplicity? IMHO Authentication should only occur once per session. In an HTTP environment like were in, we need to maintain session state ($_SESSIONS) so really we have:

1) Authentication
2) Authorization
3) Session managment

This triad allows for more modular security. In case all you need is simple authentication...you don't have the over head of authorization...and so on...

Just my two cents :)

Posted: Mon Oct 16, 2006 11:20 pm
by Luke
While I agree with some of that, for this particular purpose, I prefer this solution... any naming ideas?

Posted: Mon Oct 16, 2006 11:27 pm
by John Cartwright
Having such a generic class makes it hard to have a very specific class name.. hmm..

UserController :?:

Posted: Mon Oct 16, 2006 11:29 pm
by Luke
Why controller? it's a model

Posted: Mon Oct 16, 2006 11:42 pm
by John Cartwright
Considering your not following a naming convention specific to the layer, I personally don't see anything wrong with calling it a UserController.

Posted: Mon Oct 16, 2006 11:45 pm
by John Cartwright
I might add, I actually would call it something along the lines of UserPermission, as long as you keep the class's purpose relevant to what it is now, and not
pretty much anything having to do with the user who is viewing the site.

Posted: Mon Oct 16, 2006 11:47 pm
by Luke
Jcart wrote:Considering your not following a naming convention specific to the layer, I personally don't see anything wrong with calling it a UserController.
name's taken already...

Code: Select all

<?php
class UserController extends appController{

	public function __construct(){
	
		parent::__construct();
		$this->view->assign('sub_navigation', $this->view->render('elements/map_controls.tpl.php'));
		$this->view->assign('headercontent', $this->view->headercontent . $this->view->cssTag($this->view->wwwroot . '/styles/admin.web.css'));
	
	}
	
	public function indexAction(){
	
		$this->_forward('user', 'login');
	
	}
	
	public function loginAction(){
	
		if($this->user->isLoggedIn()) $this->_forward('user', 'home');
		
		require 'application/controller/forms/login/base.php';
		require 'application/controller/forms/login/rules.php';
		if($Form->validate()){
			if($this->user->logIn($Form->getSubmitValue('username'), $Form->getSubmitValue('password'))){
				$this->localRedirect();
			}
			$Form->setElementError('username', 'Invalid login');
		}
		$this->view->assign('form', $Form->toArray());
		$this->view->assign('sub_navigation', $this->view->render('elements/login.tpl.php'));
	
	}
	
}
?>

Posted: Tue Oct 17, 2006 12:27 am
by Chris Corbyn
Can you rename the original "User" class to "UserManager" and make this one "User"?

Posted: Tue Oct 17, 2006 12:41 am
by Luke
d11wtq wrote:Can you rename the original "User" class to "UserManager" and make this one "User"?
:D :D Perfect! thanks man!

Posted: Tue Oct 17, 2006 2:24 am
by johno
Hmm looks like your user class is pretty much doing everything.

This is how I would do it.

Code: Select all

<?php
	class User {
		private $id;
		// ...
		
		public function isLoggedIn() {
			return true;
		}
		
		public function getPermissions() {
			// permissions for user (maybe lazy loaded)
		}
	}
	
	class AnonymousUser {
		public function isLoggedIn() {
			return false;
		}
		
		public function getPermissions() {
			// default permissions
		}
	}
	
	class Permissions {
		private $permissions;
		
		// ...
		
		public function can($action) {
			return isset($permissions[$action]);
		}
	}
?>

Ok all you have to do is put have the right user in sessionpool.

Code: Select all

<?php		
	class SessionPool {
		public function __construct() {
			session_start();
			session_regenerate_id();
		}
	
		public function getUser() {
			return isset($_SESSION['User']) ? $_SESSION['User'] : $this->createAnonymous();
		}
		
		public function setUser($user) {
			$_SESSION['User'] = $user;
		}
		
		protected function createAnonymous() {
			return new AnonymousUser();
		}
	}
?>
User is logged in basicaly on a login page, right?

Code: Select all

<?php
	class LoginPage {
		public function execute($request, $session) {
			if($request->isPost()) {
				// data validation
				$finder = new UserFinder();
				$user = $finder->findByLogin($request['user'], sha1($request['password']));
				if($user !== false) {
					$session->setUser($user);
					throw new Redirect('portal.php');
				} else {
					// user not exists
				}
			}
		}
	}
?>
And now if you need user on some other page that requires permissions.

Code: Select all

<?php
	class APageThatRequiresSpecialRights {
		public function execute($request, $session) {
			if($session->getUser()->getPermissions()->can('access_this_special_page')) {
				// display page
			} else {
				// you are not allowed to see this confidential stuff...
			}
		}
	}
?>

Posted: Tue Oct 17, 2006 10:17 am
by Luke
thanks johno, I will definately use some of that... :)

Posted: Wed Oct 18, 2006 8:05 am
by neophyte
I lumped all my user stuff into a monster class once. I liked it at first but after awhile it became very limiting, hard to change and frick'n HUGE.

IMHO Hockey's dead on with this one -- split it up.