jasonhinkle / phreeze

Phreeze Framework for PHP
http://phreeze.com/
GNU Lesser General Public License v2.1
377 stars 166 forks source link

Secure example #36

Closed xtrasmal closed 11 years ago

xtrasmal commented 12 years ago

Hi there,

I'm stuck implementing the my Account class, into the SecureExample. I have been trying a lot the last couple of hours.

I have my databases set up like this: http://code.google.com/p/phreeze/wiki/AuthenticationExample

The SecureExampleController adds the ExampleUser, which constructs a array of USERS. Can you give me a pointer how I can get my Account tables inside this array?

xtrasmal commented 12 years ago

So I woke up just yet and now it starts to get more clear. Please correct me if I'm wrong:

I will create a Static USERS which hold an array of users, passwords and roles. Which is only available for my Model.In my case I have a Model called Account.

When I login, the ROUTER loads up a function in the AccountController, which loads a function inside the Account model, called Login. This function should check for matches between the user's POST data and the Array.

If there is a match. I should check the user's role and set the Static Permission.

xtrasmal commented 12 years ago

YES I got an error.

Account with primary key of Xander not found

Well, I know what I am doing wrong. I need to check on username, not on primary key. But how do I do this.

This is my Account Model class


<?php
/** @package    Teambase2::Model */

/** import supporting libraries */
require_once("DAO/AccountDAO.php");
require_once("AccountCriteria.php");
require_once("verysimple/Authentication/IAuthenticatable.php");
require_once("util/password.php");

/**
 * The Account class extends AccountDAO which provides the access
 * to the datastore.
 *
 * @package Teambase2::Model
 * @author ClassBuilder
 * @version 1.0
 */
class Account extends AccountDAO implements IAuthenticatable
{
    static $USERS;

    public function Init()
    {

    }

    public function IsAnonymous()
    {
        return $this->Username == '';
    }

    public function IsAuthorized($permission)
    {
        if ($this->Username == 'admin') return true;

        if ($this->Username == 'demo' && $permission == self::$PERMISSION_USER) return true;

        return false;
    }

    public function Login($username,$password)
    {

        $usern = $this->_phreezer->Get('Account',$username);
        $passw = $this->_phreezer->Get('Account',$password);

        if (!self::$USERS)
        {
            self::$USERS = Array(
                $usern=>password_hash($passw,PASSWORD_BCRYPT)
            );
        }   

        foreach (self::$USERS as $un=>$pw)
        {
            if ($username == $un && password_verify($password,$pw))
            {
                $this->Username = $username;
                break;
            }
        }

        return $this->Username != '';
    }   

    public function Validate()
    {
        return parent::Validate();
    }

    public function OnSave($insert)
    {
        if (!$this->Validate()) throw new Exception('Unable to Save Account: ' .  implode(', ', $this->GetValidationErrors()));
        return true;
    }

}

?>

Something goes wrong.

xtrasmal commented 12 years ago

Another error.

Class __PHP_Incomplete_Class has no unserializer in Authenticator.php at line 50

Stack trace:

#0 [internal function]: ExceptionThrower::HandleError(2, 'Class __PHP_Inc...', 'C:\wamp\www\phr...', 50, Array)
#1 C:\wamp\www\phreeze\projects\teambase2\phreeze\libs\verysimple\Authentication\Authenticator.php(50): unserialize('C:7:"Account":2...')
#2 C:\wamp\www\phreeze\projects\teambase2\phreeze\libs\verysimple\Phreeze\Controller.php(606): Authenticator::GetCurrentUser('teambase2_127_0...')
#3 C:\wamp\www\phreeze\projects\teambase2\phreeze\libs\verysimple\Phreeze\Controller.php(93): Controller->GetCurrentUser()
#4 C:\wamp\www\phreeze\projects\teambase2\phreeze\libs\verysimple\Phreeze\Dispatcher.php(89): Controller->__construct(Object(Phreezer), Object(SavantRenderEngine), NULL, Object(GenericRouter))
#5 C:\wamp\www\phreeze\projects\teambase2\index.php(28): Dispatcher::Dispatch(Object(Phreezer), Object(SavantRenderEngine), '', NULL, Object(GenericRouter))
#6 {main}

I changed my Login function to do this:

    /**
     * This login method uses hard-coded username/passwords.  This is ok for simple apps
     * but for a more robust application this would do a database lookup instead.
     * The Username is used as a mechanism to determine whether the user is logged in or
     * not
     * 
     * @see IAuthenticatable
     * @param string $username
     * @param string $password
     */
    public function Login($username,$password)
    {
        $criteria = new AccountCriteria();
        // set both the name and the _Equals properties for backwards compatibility
        $criteria->Name = $username;
        $criteria->Name_Equals = $username;
        $criteria->Password = $password;
        $criteria->Password_Equals = $password;     
        $accounts = $this->_phreezer->Query('Account', $criteria);

        if (!self::$USERS)
        {       
            foreach ($accounts as $account) 
            {
                $accUser = $account->Name;
                $accPass = $account->Password;
                    self::$USERS = Array(
                    $accUser=>password_hash($accPass,PASSWORD_BCRYPT)
                );
            }

        }

        foreach (self::$USERS as $un=>$pw)
        {
            if ($username == $un && password_verify($password,$pw))
            {
                $this->Name = $username;
                break;
            }
        }

        return $this->Name != '';
    }
xtrasmal commented 12 years ago

@jasonhinkle Ok.. I destroyed something. I removed all the code, changed the ROUTE back to your SecureExample.Login but still the error pops up. What did I kill?

xtrasmal commented 12 years ago

Switched browser, problem is gone. Caching kept the problem alive. But after another login attempt, the problem came back.

jasonhinkle commented 12 years ago

Hey you are on the right track

I'll give you two answers for your two different issues.

For the error you are getting - that is easy - you just need to do one simple thing which is inside _app_config.php - you will see a section of code that is commented and says something like "include session classes"

Basically when you save an object into the session with PHP, you have to make sure to require_once that class file on every page of your app. Because when the session starts, if the class hasn't been included already then PHP doesn't know how to de-serialize it.

So just anywhere in that area of _app_config.php put the line:

require_once "Model/Account";

That will take care of the unserialize error.

jasonhinkle commented 12 years ago

As for your other question about the static USERS array. That was only there for ExampleUser so that I could provide a simple authentication example without requiring some kind of specific database table to hold the account records. Basically that is just a hard-coded example.

The best thing to do is actually just replace ExampleUser with your own class, in this case Account, as you have already done.

But since you have a database table you don't need to store a static list of all accounts. Just do a read from the database inside your Login method instead of looking up a value in a static array.

xtrasmal commented 12 years ago

Awesome. After adding the Model/Account I now can log in. I just need to set the right permission. Thank you very much.

xtrasmal commented 12 years ago

Underneath I will post two files. The AccountController.php and Account.php. Can you tell me what needs to be changed so that the permission is getting stored in the session?

_AccountController.php_

<?php
/** @package    TEAMBASE::Controller */

/** import supporting libraries */
require_once("AppBaseController.php");
require_once("Model/Account.php");

/**
 * AccountController is the controller class for the Account object.  The
 * controller is responsible for processing input from the user, reading/updating
 * the model as necessary and displaying the appropriate view.
 *
 * @package TEAMBASE::Controller
 * @author ClassBuilder
 * @version 1.0
 */
class AccountController extends AppBaseController
{

    /**
     * Override here for any controller-specific functionality
     *
     * @inheritdocs
     */
    protected function Init()
    {
        parent::Init();

        // TODO: add controller-wide bootstrap code

        // TODO: if authentiation is required for this entire controller, for example:
        // $this->RequirePermission(ExampleUser::$PERMISSION_USER,'SecureExample.LoginForm');
    }

    /**
     * MY STUFF
     */

    /**
     * Process the login, create the user session and then redirect to 
     * the appropriate page
     */
    public function Login()
    {
        $account = new Account($this->Phreezer);

        if ($account->Login(RequestUtil::Get('username'), RequestUtil::Get('password')))
        {
            // login success
            $this->SetCurrentUser($account);
            $this->Redirect('Account.UserPage');
        }
        else
        {
            $this->Redirect('Account.LoginForm','Unknown username/password combination');
        }
    }

    /**
     * This page requires ExampleUser::$PERMISSION_USER to view
     */
    public function UserPage()
    {
        $this->RequirePermission(P_SUPERUSER, 
                'Account.LoginForm', 
                'Login is required to access the secure user page',
                'You do not have permission to access the secure user page');

        $this->Assign("currentUser", $this->GetCurrentUser());

        $this->Assign('page','userpage');
        $this->Render("SecureExample");
    }

    /**
     * This page requires ExampleUser::$PERMISSION_ADMIN to view
     */
    public function AdminPage()
    {
        $this->RequirePermission(P_SUPERUSER, 
                'Account.LoginForm', 
                'Login is required to access the admin page',
                'Admin permission is required to access the admin page');

        $this->Assign("currentUser", $this->GetCurrentUser());

        $this->Assign('page','adminpage');
        $this->Render("SecureExample");
    }

    /**
     * Display the login form
     */
    public function LoginForm()
    {
        $this->Assign("currentUser", $this->GetCurrentUser());

        $this->Assign('page','login');
        $this->Render("SecureExample");
    }

    /**
     * Clear the user session and redirect to the login page
     */
    public function Logout()
    {
        $this->ClearCurrentUser();
        $this->Redirect("Account.LoginForm","You are now logged out");
    }

    /**
     * END MY STUFF
     */

    /**
     * Displays a list view of campaign objects
     */
    public function ListView()
    {
        $this->Render();
    }

    /**
     * API Method queries for campaign records and render as JSON
     */
    public function Query()
    {
        try
        {
            $criteria = new AccountCriteria();

            // TODO: this is generic query filtering based only on criteria properties
            foreach (array_keys($_REQUEST) as $prop)
            {
                $prop_normal = ucfirst($prop);
                $prop_equals = $prop_normal.'_Equals';

                if (property_exists($criteria, $prop_normal))
                {
                    $criteria->$prop_normal = RequestUtil::Get($prop);
                }
                elseif (property_exists($criteria, $prop_equals))
                {
                    // this is a convenience so that the _Equals suffix is not needed
                    $criteria->$prop_equals = RequestUtil::Get($prop);
                }
            }

            $output = new stdClass();

            // if a sort order was specified then specify in the criteria
            $output->orderBy = RequestUtil::Get('orderBy');
            $output->orderDesc = RequestUtil::Get('orderDesc') != '';
            if ($output->orderBy) $criteria->SetOrder($output->orderBy, $output->orderDesc);

            $page = RequestUtil::Get('page');

            if ($page != '')
            {
                // if page is specified, use this instead (at the expense of one extra count query)
                $pagesize = $this->GetDefaultPageSize();

                $accounts = $this->Phreezer->Query('Account',$criteria)->GetDataPage($page, $pagesize);
                $output->rows = $accounts->ToObjectArray(true,$this->SimpleObjectParams());
                $output->totalResults = $accounts->TotalResults;
                $output->totalPages = $accounts->TotalPages;
                $output->pageSize = $accounts->PageSize;
                $output->currentPage = $accounts->CurrentPage;
            }
            else
            {
                // return all results
                $accounts = $this->Phreezer->Query('Account',$criteria);
                $output->rows = $accounts->ToObjectArray(true, $this->SimpleObjectParams());
                $output->totalResults = count($output->rows);
                $output->totalPages = 1;
                $output->pageSize = $output->totalResults;
                $output->currentPage = 1;
            }

            $this->RenderJSON($output, $this->JSONPCallback());
        }
        catch (Exception $ex)
        {
            $this->RenderExceptionJSON($ex);
        }
    }

    /**
     * API Method retrieves a single campaign record and render as JSON
     */
    public function Read()
    {
        try
        {
            $pk = $this->GetRouter()->GetUrlParam('id');
            $account = $this->Phreezer->Get('Account',$pk);
            $this->RenderJSON($account, $this->JSONPCallback(), true, $this->SimpleObjectParams());
        }
        catch (Exception $ex)
        {
            $this->RenderExceptionJSON($ex);
        }
    }

    /**
     * API Method inserts a new campaign record and render response as JSON
     */
    public function Create()
    {
        try
        {
            $json = json_decode(RequestUtil::GetBody());

            if (!$json)
            {
                throw new Exception('The request body does not contain valid JSON');
            }

            $account = new Account($this->Phreezer);

            // TODO: any fields that should not be inserted by the user should be commented out

            // this is an auto-increment.  uncomment if updating is allowed
            // $account->Id = $this->SafeGetVal($json, 'id');

            $account->Name = $this->SafeGetVal($json, 'name');
            $account->Salt = $this->SafeGetVal($json, 'salt');
            $account->Password = $this->SafeGetVal($json, 'password');
            $account->Nicename = $this->SafeGetVal($json, 'nicename');
            $account->FirstName = $this->SafeGetVal($json, 'firstName');
            $account->LastName = $this->SafeGetVal($json, 'lastName');
            $account->Email = $this->SafeGetVal($json, 'email');
            $account->RoleId = $this->SafeGetVal($json, 'roleId');

            $account->Validate();
            $errors = $account->GetValidationErrors();

            if (count($errors) > 0)
            {
                $this->RenderErrorJSON('Please check the form for errors',$errors);
            }
            else
            {
                $account->Save();
                $this->RenderJSON($account, $this->JSONPCallback(), true, $this->SimpleObjectParams());
            }

        }
        catch (Exception $ex)
        {
            $this->RenderExceptionJSON($ex);
        }
    }

    /**
     * API Method updates an existing campaign record and render response as JSON
     */
    public function Update()
    {
        try
        {
            $json = json_decode(RequestUtil::GetBody());

            if (!$json)
            {
                throw new Exception('The request body does not contain valid JSON');
            }

            $pk = $this->GetRouter()->GetUrlParam('id');
            $account = $this->Phreezer->Get('Account',$pk);

            // TODO: any fields that should not be updated by the user should be commented out

            // this is a primary key.  uncomment if updating is allowed
            // $account->Id = $this->SafeGetVal($json, 'id', $account->Id);

            $account->Name = $this->SafeGetVal($json, 'name', $account->Name);
            $account->Salt = $this->SafeGetVal($json, 'salt', $account->Salt);
            $account->Password = $this->SafeGetVal($json, 'password', $account->Password);
            $account->Nicename = $this->SafeGetVal($json, 'nicename', $account->Nicename);
            $account->FirstName = $this->SafeGetVal($json, 'firstName', $account->FirstName);
            $account->LastName = $this->SafeGetVal($json, 'lastName', $account->LastName);
            $account->Email = $this->SafeGetVal($json, 'email', $account->Email);
            $account->RoleId = $this->SafeGetVal($json, 'roleId', $account->RoleId);

            $account->Validate();
            $errors = $account->GetValidationErrors();

            if (count($errors) > 0)
            {
                $this->RenderErrorJSON('Please check the form for errors',$errors);
            }
            else
            {
                $account->Save();
                $this->RenderJSON($account, $this->JSONPCallback(), true, $this->SimpleObjectParams());
            }

        }
        catch (Exception $ex)
        {

            $this->RenderExceptionJSON($ex);
        }
    }

    /**
     * API Method deletes an existing campaign record and render response as JSON
     */
    public function Delete()
    {
        try
        {
            // TODO: if a soft delete is prefered, change this to update the deleted flag instead of hard-deleting

            $pk = $this->GetRouter()->GetUrlParam('id');
            $account = $this->Phreezer->Get('Account',$pk);

            $account->Delete();

            $output = new stdClass();

            $this->RenderJSON($output, $this->JSONPCallback());

        }
        catch (Exception $ex)
        {
            $this->RenderExceptionJSON($ex);
        }
    }
}

?>

_Account.php_

<?php
/** @package    Teambase2::Model */

/** import supporting libraries */
require_once("DAO/AccountDAO.php");
require_once("AccountCriteria.php");
require_once("verysimple/Authentication/IAuthenticatable.php");
require_once("util/password.php");

define("P_READ",1);
define("P_WRITE",2);
define("P_UPDATE",4);
define("P_ADMIN",8);
define('P_SUPERUSER',16);

/**
 * The Account class extends AccountDAO which provides the access
 * to the datastore.
 *
 * @package Teambase2::Model
 * @author ClassBuilder
 * @version 1.0
 */
class Account extends AccountDAO implements IAuthenticatable
{
    static $USERS;
    static $PERMISSION_USER;
    /**
     * Initialize the array of users.  Note, this is only done this way because the 
     * users are hard-coded for this example.  In your own code you would most likely
     * do a single lookup inside the Login method
     */
    public function Init()
    {

    }

    function GetCurrentUser(){
        return $this->Name;
    }

    public function GetRole()
    {
        $obj = new stdClass();
        $obj->Permission = $this->RoleId;  // you probably want to get this from your account table instead
        return $obj;
    }   
    /**
     * Returns true if the user is anonymous (not logged in)
     * @see IAuthenticatable
     */
    public function IsAnonymous()
    {
        return (!$this->Id);
    }

    /**
     * This is a hard-coded way of checking permission.  A better approach would be to look up
     * this information in the database or base it on the account type
     * 
     * @see IAuthenticatable
     * @param int $permission
     */
    public function IsAuthorized($permission)
    {
        if ($this->IsAnonymous())
        {
            return false;
        }

        return (($this->GetRole()->Permission && $permission) > 0);
    }

    /**
     * This login method uses hard-coded username/passwords.  This is ok for simple apps
     * but for a more robust application this would do a database lookup instead.
     * The Username is used as a mechanism to determine whether the user is logged in or
     * not
     * 
     * @see IAuthenticatable
     * @param string $username
     * @param string $password
     */
    public function Login($username,$password)
    {
        $criteria = new AccountCriteria();
        // set both the name and the _Equals properties for backwards compatibility
        $criteria->Name = $username;
        $criteria->Name_Equals = $username;
        $criteria->Password = $password;
        $criteria->Password_Equals = $password;     
        $accounts = $this->_phreezer->Query('Account', $criteria);

        if (!self::$USERS)
        {       
            foreach ($accounts as $account) 
            {
                $accUser = $account->Name;
                $accPass = $account->Password;
                    self::$USERS = Array(
                    $accUser=>password_hash($accPass,PASSWORD_BCRYPT)
                );
            }

        }

        foreach (self::$USERS as $un=>$pw)
        {
            if ($username == $un && password_verify($password,$pw))
            {
                $this->Name = $username;
                break;
            }
        }
        $this->getRole();
        return $this->Name != '';
    }   

    /**
     * Override default validation
     * @see Phreezable::Validate()
     */
    public function Validate()
    {
        // example of custom validation
        // $this->ResetValidationErrors();
        // $errors = $this->GetValidationErrors();
        // if ($error == true) $this->AddValidationError('FieldName', 'Error Information');
        // return !$this->HasValidationErrors();

        return parent::Validate();
    }

    /**
     * @see Phreezable::OnSave()
     */
    public function OnSave($insert)
    {
        // the controller create/update methods validate before saving.  this will be a
        // redundant validation check, however it will ensure data integrity at the model
        // level based on validation rules.  comment this line out if this is not desired
        if (!$this->Validate()) throw new Exception('Unable to Save Account: ' .  implode(', ', $this->GetValidationErrors()));

        // OnSave must return true or eles Phreeze will cancel the save operation
        return true;
    }

}

?>
jasonhinkle commented 12 years ago

The only issue I think is your Account->Login method. Here's a login method from one of my own apps. My app uses password hashing so it does the lookup of accounts this way, which yours may do some alternative way. But that part is up to you - this should just show what type of value to return.

I haven't really provided this because Phreeze doesn't care how you actually implement your Login method, and I don't want to enforce or even encourage any particular way of storing accounts and passwords (because one goal of the framework is to allow you to implement things to fit within your own schema). But, I understand that it helps to have some guidance. So, please don't take this as the law on how it has to be done, but rather this is one example of how you can do authentication

    /**
     * Validate the given username/password.  If successful then $this becomes the
     * authenticated user and is returned
     *
     * @param string $username
     * @param string $password
     * @return Account or NULL
     */
    function Login($username, $password)
    {

        $result = null;

        if ($username == "" || $password == "") return null;

        // to begin, query for all users with a matching username.  we cannot check at this point
        // if the password matches or not because it is crypted with a salt
        $criteria = new AccountCriteria();
        $criteria->Username_Equals = $username;

        $accounts = $this->_phreezer->Query("Account", $criteria)->ToObjectArray();

        // If we have any objects in this array then we know a correct username was enterd,
        // however we do not know if the password is correct until we verify the hash
        foreach ($accounts as $possibleAccount)
        {
            if ( password_verify($password,$possibleAccount->Password) )
            {

                // this is a successful login, the username and password matches.
                // what we are doing here is copying all of the properties from $possibleAccount
                // into $this.  we can't do $this = $possibleAccount because PHP would throw
                // exception. so instead we just clone all of the properties from $possibleAccount
                // into $this using a Phreezable method 'LoadFromObject'
                $this->LoadFromObject($possibleAccount);

                // on success this method will return a reference to itself
                $result = $this;
                break;
            }
        }

        // this will either be a reference to itself (which will evaluate as true)
        // or it will be null (which will evaluate as false)
        return $result;

    }
xtrasmal commented 12 years ago

Thanks, problem solved. I had to use the LoadFromObject method.

    public function Login($username,$password)
    {
        $criteria = new AccountCriteria();
        // set both the name and the _Equals properties for backwards compatibility
        $criteria->Name = $username;
        $criteria->Name_Equals = $username;
        $accounts = $this->_phreezer->Query('Account', $criteria);

        if (!self::$USERS)
        {       
            foreach ($accounts as $account) 
            {
                $accUser = $account->Name;
                $accPass = $account->Password;
                    self::$USERS = Array(
                    $accUser=>password_hash($accPass,PASSWORD_BCRYPT)
                );
                    $this->LoadFromObject($account);
            }

        }

        foreach (self::$USERS as $un=>$pw)
        {
            if ($username == $un && password_verify($password,$pw))
            {

                $this->Name = $username;
                // on success this method will return a reference to itself
                $result = $this;        
                break;

            }
        }

        return $this->Name != '';
    }   
jasonhinkle commented 11 years ago

cleaning up tracker...

myrond commented 11 years ago

what xtrasmal posted was extremely helpful; is it possible to have in builder to have a option to throw out a better example then the ExampleUser class? I.E. if builder sees a table like described in https://code.google.com/p/phreeze/wiki/AuthenticationExample to have a checkbox (or something) which says "authentication compatible table detected, implement authentication?". That would be pretty cool. (I can post and create a different ticket).

jasonhinkle commented 11 years ago

I've gone back and forth on that one because I don't want to enforce any particular kind of schema that people have to use. But at the same time, I know it's really helpful and often people don't necessarily mind having to make their schema one way or another.

Another option would be to just create a zip file that has the relevant model files and a SQL script to run and create the account tables. Then people can download that as an "add-in" or something to their app..?

myrond commented 11 years ago

I was able to successfully deploy authentication; however just consider this a user feedback; deploying authentication learning curve was a LOT higher than other parts of your framework. It is very possible; I just thought I would leave some feedback that I think it could be easier with a SQL sample included. This particular ticket helped a lot for me understanding and helping me in deploying authentication. Take for what its worth...

jasonhinkle commented 11 years ago

Cool, yea I appreciate it. The whole point of the framework is to make things easier so I'm always interested. Authentication is one of those things - I've seen other frameworks where you have to have your DB set up exactly the way they want, and I don't like that myself! So I try to never push it onto others with Phreeze. I prefer creating Interfaces for people to implement.

But authentication is a very common use-case, and it can be quite difficult to set up. So I think at the very least I'll try to put together a sample app or an add-in that can be used.

On a related note, I'm going to be adding in CSRF example code too, which goes somewhat hand-in-hand with authentication.