Skip to content

Commit

Permalink
Dev: authentication, added comments to untangle the "spaghettis alle …
Browse files Browse the repository at this point in the history
…plugin salsa".
  • Loading branch information
LouisGac committed Oct 17, 2016
1 parent affbf6e commit 62fb627
Showing 1 changed file with 31 additions and 24 deletions.
55 changes: 31 additions & 24 deletions application/controllers/admin/authentication.php
Expand Up @@ -31,10 +31,11 @@ class Authentication extends Survey_Common_Action
/**
* Show login screen and parse login data
* Will redirect or echo json depending on ajax call
* @return void
* This function is called while accessing the login page: index.php/admin/authentication/sa/login

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Oct 17, 2016

Contributor

It still returns void, no?

This comment has been minimized.

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Oct 18, 2016

Collaborator

Personally : i like to return void : more clear the returned value is not used.

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Oct 18, 2016

Contributor

I agree with Denis on this one. Otherwise, it seems like phpdoc is forgotten (which is usually the case in LS).

This comment has been minimized.

Copy link
@LouisGac

LouisGac Oct 18, 2016

Contributor

the real problem is that no function should return void.
But feel free to put it back if you want.

This comment has been minimized.

Copy link
@Shnoulle
*/
public function index()
{
// The page should be shown only for non logged in users
$this->_redirectIfLoggedIn();

// Result can be success, fail or data for template
Expand Down Expand Up @@ -77,59 +78,65 @@ public function index()

/**
* Prepare login and return result
* It checks if the authdb plugin is registered and active
* @return array Either success, failure or plugin data (used in login form)
*/
public static function prepareLogin()
{
$aData = array();

// Make sure after first run / update the authdb plugin is registered and active
// it can not be deactivated
if (!class_exists('Authdb', false)) {
$plugin = Plugin::model()->findByAttributes(array('name'=>'Authdb'));
if (!$plugin) {
$plugin = new Plugin();
$plugin->name = 'Authdb';
$plugin->active = 1;
$plugin->save();
App()->getPluginManager()->loadPlugin('Authdb', $plugin->id);
} else {
$plugin->active = 1;
$plugin->save();
}
}

// In Authdb, the plugin event "beforeLogin" checks if the url param "onepass" is set
// if yes, it will call AuthPluginBase::setAuthPlugin to set to true the plugin private parameter "_stop", so the form will not be displayed
// @see: application/core/plugins/Authdb/Authdb.php: function beforeLogin()
$beforeLogin = new PluginEvent('beforeLogin');
$beforeLogin->set('identity', new LSUserIdentity('', ''));

App()->getPluginManager()->dispatchEvent($beforeLogin);

/* @var $identity LSUserIdentity */
$identity = $beforeLogin->get('identity');
$identity = $beforeLogin->get('identity'); // Why here?
// If the plugin private parameter "_stop" is false and the login form has not been submitted: render the login form
if (!$beforeLogin->isStopped() && is_null(App()->getRequest()->getPost('login_submit')) )
{
// First step: set the value of $aData['defaultAuth']
// This variable will be used to select the default value of the Authentication method selector
// which is shown only if there is more than one plugin auth on...
// @see application/views/admin/authentication/login.php

// First it checks if the current plugin force the authentication default value...
// NB: A plugin SHOULD NOT be able to over pass the configuration file
// @see: http://img.memecdn.com/knees-weak-arms-are-heavy_c_3011277.jpg

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Oct 17, 2016

Contributor

?

if (!is_null($beforeLogin->get('default'))) {

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Oct 17, 2016

Contributor

This is btw a good use-case to factor out code into its own function. Then the docs can belong to that function instead of hanging mid air. Documenting function args is a good way to document information flow in the system, and function names are part of that.

This comment has been minimized.

Copy link
@LouisGac

LouisGac Oct 18, 2016

Contributor

First step: comments.
Second step: refactor creating subfunctions.
Third step: move the code in the right place (in this case, it would imply to rewrite the plugin system to make it extends some basic Yii classes).

I've done that hundreds of time since I'm here.

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Oct 18, 2016

Contributor

👍

$aData['defaultAuth'] = $beforeLogin->get('default');
}
else {
// THen, it checks if the the user set a different default plugin auth in application/config/config.php
// eg: 'config'=>array()'debug'=>2,'debugsql'=>0, 'default_displayed_auth_method'=>'muh_auth_method')
if (App()->getPluginManager()->isPluginActive(Yii::app()->getConfig('default_displayed_auth_method'))) {
$aData['defaultAuth'] = Yii::app()->getConfig('default_displayed_auth_method');
}
else {
}else {
$aData['defaultAuth'] = 'Authdb';
}
}

// Call the plugin method newLoginForm
// For Authdb: @see: application/core/plugins/Authdb/Authdb.php: function newLoginForm()
$newLoginForm = new PluginEvent('newLoginForm');
App()->getPluginManager()->dispatchEvent($newLoginForm);
App()->getPluginManager()->dispatchEvent($newLoginForm); // inject the HTML of the form inside the private varibale "_content" of the plugin
$aData['summary'] = self::getSummary('logout');
$aData['pluginContent'] = $newLoginForm->getAllContent();
$aData['pluginContent'] = $newLoginForm->getAllContent(); // Retreives the private varibale "_content" , and parse it to $aData['pluginContent'], which will be rendered in application/views/admin/authentication/login.php
}
else
{
// The form has been submited, or the plugin has been stoped (so normally, the value of login/password are available)

// Handle getting the post and populating the identity there
$authMethod = App()->getRequest()->getPost('authMethod', $identity->plugin);
$authMethod = App()->getRequest()->getPost('authMethod', $identity->plugin); // If form has been submitted, $_POST['authMethod'] is set, else $identity->plugin should be set, ELSE: TODO error
$identity->plugin = $authMethod;

// Call the function afterLoginFormSubmit of the plugin.
// For Authdb, it calls AuthPluginBase::afterLoginFormSubmit()
// which set the plugin's private variables _username and _password with the POST informations if it's a POST request else it does nothing
$event = new PluginEvent('afterLoginFormSubmit');
$event->set('identity', $identity);
App()->getPluginManager()->dispatchEvent($event, array($authMethod));
Expand Down

0 comments on commit 62fb627

Please sign in to comment.