views:

212

answers:

3

I've tried to create a login widget that can be included at will in any action that is outside the content of the application that requires login. My thought here was that I wanted a DRY-approach for rendering the login box with any error messages from a previous login attempt. To enable this I utilized the FlashMessenger for saving the result of a failed login-attempt. The widget also uses the Post/Redirect/Get-pattern to avoid the problem of posting data multiple times when using the browser back-button.

Now, the problem is that I would rather not use the Zend_View_Helper_Action since that helper clones the request and creates another run of the dispatch-loop, which is pretty resource intensive. So, by looking at the code below, could you give me some advice on how to refactor the code such that:

  1. The widget can be included in an arbitrary view script
  2. Results from a previous login attempt is rendered
  3. The widget does not invoke a run in the dispatch-loop

Currently, the login widget is rendered by calling, in the view scripts:

echo $this->action('auth', 'login-widget');

AuthController.php:

class AuthController extends Zend_Controller_Action {    
    // This method is invoked by Zend_View_Helper_Action to render the
    // login widget
    public function loginWidgetAction () {
        $flashMessenger = $this->_helper->flashMessenger->setNamespace('login');
        $this->view->messages = $flashMessenger->getMessages();
    }

    public function loginAction () {
        if($this->getRequest()->isPost()) {
            $result = Auth::doLogin($this->getRequest()->getPost());
            if($result->isValid()) {
                $this->_redirect('user');
            } 
            else {
                $flashMessenger = $this->_helper->flashMessenger->
                                                  setNamespace('login');
                foreach($result->getMessages() as $message) {
                    $flashMessenger->addMessage($message);    
                }
            }
        }
        // This will be changed to redirect either to the index page,
        // or the page the request originated from if available.
        $this->_redirect('');
    }
    [...]
}

/models/Auth.php:

/**
 * Model for encapsulating the actions that deals with authentication,
 * such as registering and activating users, as well as logging in and
 * logging out.
 * @todo: Refactor this to remove static methods
 */
class Auth {

    /**
     * 
     * @return Zend_Auth_Result
     */
    public static function doLogin ($credentials) {
        $authAdapter = new Auth_Adapter_DbTable(
            Zend_Db_Table::getDefaultAdapter(),
            'Users',
            'username',
            'pwdHash',
            'SHA1(CONCAT(?, salt))'
        );
        $authAdapter->setIdentity($credentials['username']);
        $authAdapter->setCredential($credentials['password']);
        $auth = Zend_Auth::getInstance();
        return $auth->authenticate($authAdapter);
}
[...]
}

/models/Auth/Adapter/DbTable.php:

class Auth_Adapter_DbTable extends Zend_Auth_Adapter_DbTable {    

    /**
     * authenticate() - defined by Zend_Auth_Adapter_Interface.  This method 
     * is called to attempt an authenication.  Previous to this call, this
     * adapter would have already been configured with all necessary 
     * information to successfully connect to a database table and attempt
     * to find a record matching the provided identity.
     *
     * @throws Zend_Auth_Adapter_Exception if answering the authentication 
     * query is impossible
     * @see library/Zend/Auth/Adapter/Zend_Auth_Adapter_DbTable#authenticate()
     * @return MedU_Auth_Result
     */
    public function authenticate() {
        return parent::authenticate();
    }

    /**
     * _authenticateValidateResult() - This method attempts to validate that
     * the record in the result set is indeed a record that matched the 
     * identity provided to this adapter.
     * 
     * Additionally it checks that the user account is activated. 
     *
     * @param array $resultIdentity
     * @return MedU_Auth_Result
     */
    protected function _authenticateValidateResult($resultIdentity)
    {
        $result = parent::_authenticateValidateResult($resultIdentity);
        if(!$result->isValid()) { return $result; }

        $this->_checkAccountIsActivated($resultIdentity);
        $this->_checkAccountIsSuspended($resultIdentity);

        // Overwrite the username supplied by the user and instead
        // use the name supplied upon registration, i.e if the
        // user signs in as uSERNAME and registered as Username,
        // the identity is Username
        $this->_authenticateResultInfo['identity'] = 
            $resultIdentity[$this->_identityColumn]; 

        return $this->_authenticateCreateAuthResult();
    }

    protected function _checkAccountIsActivated ($resultIdentity) {
        if(!$resultIdentity['activated']) {
            $this->_authenticateResultInfo['code'] = 
                MedU_Auth_Result::FAILURE_NOT_ACTIVATED;
            $this->_authenticateResultInfo['messages'] = 
                array('The account has not yet been activated. 
                       Please click on the link provided in the 
                       activation email.');
        }
    }

    protected function _checkAccountIsSuspended ($resultIdentity) {
        if($resultIdentity['suspended']) {
            $this->_authenticateResultInfo['code'] = 
                MedU_Auth_Result::FAILURE_SUSPENDED;
            $this->_authenticateResultInfo['messages'] = 
                  array('The account has been suspended. 
                         If you feel this is a mistake, 
                         please contact our support: [email protected]');
        }
    }

    /**
     * _authenticateCreateAuthResult() - This method creates a 
     * MedU_Auth_Result object from the information that has 
     * been collected during the authenticate() attempt.
     *
     * @return MedU_Auth_Result
     */
    protected function _authenticateCreateAuthResult()
    {
        return new MedU_Auth_Result(
            $this->_authenticateResultInfo['code'],
            $this->_authenticateResultInfo['identity'],
            $this->_authenticateResultInfo['messages']
           );
    }
}

/views/scripts/auth/partials/login-widget.phtml

// The fetchForm-view helper returns a Zend_Form object. 
// The form definition (xml) is attached below in the
// code box below this one.
<div class="box">
    <h3>Login</h3>
    <?php if($this->messages) : ?>
            <ul class="errors">
                <?php foreach($this->messages as $message) : ?>
                    <li><?php echo $message ?></li>

                <?php endforeach; ?>
            </ul>
    <?php endif; ?>
    <div>
        <?php echo $this->fetchForm('user', 'login') ?>
    </div>
</div>

/application/configs/forms.xml

// Configuration of the Login form. 'user' and 'login' are 
// just keys for the view helper to return the correct form
<forms>
    <user>
        <login>
            <action value="/auth/login" />
            <method value="post" />
            <elements>
                <username>
                    <type value="text"></type>
                    <options>
                        <label value="Username" />
                        <required value="true" />
                        <validators>
                            <alnum validator="alnum" />
                            <strlen validator="StringLength">
                                <options min="6" max="45" />
                            </strlen>
                        </validators>
                    </options>
                </username>

                <password>
                    <type value="password" />
                    <options>
                        <label value="Password" />
                        <required value="true" />
                        <validators>
                            <strlen validator="StringLength">
                                <options min="6" max="20" />
                            </strlen>
                        </validators>
                    </options>
                </password>

                <submit>
                    <type value="submit" />
                    <options>
                        <label value="Log in" />
                    </options>
                </submit>
            </elements>
    </login>
A: 

Maybe you should use ViewHelper to render form and ActionController for authentication? This frees you from second dispatch-loop. I can show you some little example, if you want.

P.S. You sure need validators for form? I don`t see any use in your example.

Maxim
I probably don't need any validators, and I don't use them at the moment. I just added them in while configuring the form, but you're right. I should probably take them out if I find no use for them.
PatrikAkerstrand
+1  A: 

As far as I understood your solution, your current call to the Zend_View_Helper_Action helper does nothing more than rendering the login form. Why don't you write your on view helper that retrieves the login-related messages from the flash-messenger and renders the login form?

EDIT:

  1. You can retrieve the flash-messenger from its static broker:

    $flashMessenger = Zend_Controller_Action_HelperBroker::getStaticHelper('FlashMessenger');
    $flashMessenger->setNamespace('login');
    $messages = $flashMessenger->getMessages();
    
  2. You don't need to instatiate a new flash-messenger in your view helper, you just can retrieve the current instance of it (see 1.). From a design point of view I'd say that it's absolutely feasible to retrieve application components within your view helpers (the Zend_View_Helper_Url for example retrieves the router from the static front-controller). To reduce coupling and to implement a setter-dependency-injection-pattern I'd suggest the following (Zend_View_Helper_Translate uses a similar pattern to retrieve the translator):

    class My_View_Helper_Login extends Zend_View_Helper_Abstract
    {
        // [...]
        /**
         * @var Zend_Controller_Action_Helper_FlashMessenger
         */
        protected $_flash;
        /**
         * @param Zend_Controller_Action_Helper_FlashMessenger $flash
         * @return My_View_Helper_Login Provides a fluent interface
         */
        public function setFlashMessenger(Zend_Controller_Action_Helper_FlashMessenger $flash)
        {
            $this->_flash = $flash;
            return $this;
        }
        /**
         * @return Zend_Controller_Action_Helper_FlashMessenger
         */
        public function getFlashMessenger()
        {
            if ($this->_flash === null) {
                $this->_flash = Zend_Controller_Action_HelperBroker::getStaticHelper('FlashMessenger');
            }
            return $this->_flash;
        }
        /**
         *
         */
        public function login()
        {
            $flashMessenger = $this->getFlashMessenger();
            $flashMessenger->setNamespace('login');
            $messages = $flashMessenger->getMessages();
            // [...]
         }
    }
    
  3. Not when the flash-messenger provides the functionality you need. If you need to distinguish between error, informational and success messages for example a home-grown solution would perhaps be the better way.

Stefan Gehrig
It does a little more than that. It also fetches the error messages and pushes them into the view. I thought of using a view-helper instead of a controller action, but I hesitated on these points:* Is it correct to instantiate a Zend_Controller_Action_Helper in a view helper? (The FlashMessenger).* How do I instantiate it in a proper way?* Should I build my own error passing mechanism instead of utilizing the flash messenger?
PatrikAkerstrand
Answes edited with responses to the questions provided.
Stefan Gehrig
I think you forgot either a ! or empty($this->_flash) in getFlashMessenger. Other than that, this solution works just as well as using the action view helper. I like your idea of a dependency injector pattern, even though it might be overkill in this scenario. Thanks for your help and ideas!
PatrikAkerstrand
Corrected mistake in code.
Stefan Gehrig
A: 

Another addition is that you can use FlashMessenger to store ANY data. So you can do

$flash->addMessage(array('status'=>'error','message'=> 'oops'));

and then in foreach use

foreach($messages as $message){
echo '<div class="message '. $message['status'] .'">' . $message['message'] . '</div>'
}
Tomáš Fejfar