From 941ce6d68ed507987c0a2615c977e04dedfb11c6 Mon Sep 17 00:00:00 2001 From: Matthias Jentsch Date: Wed, 20 Nov 2013 12:01:40 +0100 Subject: [PATCH 1/3] Fix bug that caused ajax-request to override values written to the session Authentication/Session.php and its Subclasses do not have a open/closed -state anymore. Read will refresh the session, write will always write the changes, and opening/closing will be handled internally. refs #5101 --- .../controllers/AuthenticationController.php | 17 +- .../controllers/PreferenceController.php | 9 - library/Icinga/Application/Web.php | 7 +- library/Icinga/Authentication/Manager.php | 9 +- library/Icinga/Authentication/PhpSession.php | 157 ++++-------------- library/Icinga/Authentication/Session.php | 18 +- library/Icinga/User.php | 25 ++- .../Web/Controller/ActionController.php | 16 +- .../Icinga/Authentication/PhpSessionTest.php | 53 +++--- 9 files changed, 80 insertions(+), 231 deletions(-) diff --git a/application/controllers/AuthenticationController.php b/application/controllers/AuthenticationController.php index b9a3e1216..be6a4a463 100644 --- a/application/controllers/AuthenticationController.php +++ b/application/controllers/AuthenticationController.php @@ -48,15 +48,6 @@ class AuthenticationController extends ActionController */ protected $requiresAuthentication = false; - /** - * This controller modifies the session - * - * @var bool - * - * @see \Icinga\Web\Controller\ActionController::$modifiesSession - */ - protected $modifiesSession = true; - /** * Log into the application */ @@ -69,9 +60,7 @@ class AuthenticationController extends ActionController $this->view->form->setRequest($this->_request); $this->view->title = "Icinga Web Login"; try { - $auth = AuthManager::getInstance(null, array( - 'writeSession' => $this->modifiesSession - )); + $auth = AuthManager::getInstance(); if ($auth->isAuthenticated()) { $this->redirectNow('index?_render=body'); @@ -104,9 +93,7 @@ class AuthenticationController extends ActionController public function logoutAction() { $this->_helper->layout->setLayout('inline'); - $auth = AuthManager::getInstance(null, array( - 'writeSession' => $this->modifiesSession - )); + $auth = AuthManager::getInstance(); $auth->removeAuthorization(); $this->redirectToLogin(); } diff --git a/application/controllers/PreferenceController.php b/application/controllers/PreferenceController.php index b7d64ec4f..a8245e136 100644 --- a/application/controllers/PreferenceController.php +++ b/application/controllers/PreferenceController.php @@ -40,15 +40,6 @@ use \Icinga\Form\Preference\GeneralForm; class PreferenceController extends BasePreferenceController { - /** - * This controller modifies the session - * - * @var bool - * - * @see \Icinga\Web\Controller\ActionController::$modifiesSession - */ - protected $modifiesSession = true; - /** * Create tabs for this preference controller * diff --git a/library/Icinga/Application/Web.php b/library/Icinga/Application/Web.php index a5d5da7d2..756316af8 100644 --- a/library/Icinga/Application/Web.php +++ b/library/Icinga/Application/Web.php @@ -232,12 +232,7 @@ class Web extends ApplicationBootstrap */ private function setupUser() { - $authenticationManager = AuthenticationManager::getInstance( - null, - array( - 'writeSession' => true - ) - ); + $authenticationManager = AuthenticationManager::getInstance(); if ($authenticationManager->isAuthenticated() === true) { $user = $authenticationManager->getUser(); diff --git a/library/Icinga/Authentication/Manager.php b/library/Icinga/Authentication/Manager.php index b089ea015..9445581f7 100644 --- a/library/Icinga/Authentication/Manager.php +++ b/library/Icinga/Authentication/Manager.php @@ -121,16 +121,10 @@ class Manager } if (!isset($options['sessionClass'])) { - $this->session = new PhpSession($config->session); + $this->session = new PhpSession(); } else { $this->session = $options['sessionClass']; } - - if (isset($options['writeSession']) && $options['writeSession'] === true) { - $this->session->read(true); - } else { - $this->session->read(); - } } /** @@ -353,6 +347,7 @@ class Manager return false; } + $this->user->sendMessage("Hallo ich bin nachricht."); if ($persist == true) { $this->persistCurrentUser(); $this->session->write(); diff --git a/library/Icinga/Authentication/PhpSession.php b/library/Icinga/Authentication/PhpSession.php index 14fe12525..fea5b3663 100644 --- a/library/Icinga/Authentication/PhpSession.php +++ b/library/Icinga/Authentication/PhpSession.php @@ -51,21 +51,7 @@ class PhpSession extends Session * * @var string */ - const SESSION_NAME = 'Icinga2Web'; - - /** - * Flag if session is open - * - * @var bool - */ - private $isOpen = false; - - /** - * Flag if session is flushed - * - * @var bool - */ - private $isFlushed = false; + private $sessionName = 'Icingaweb2'; /** * Configuration for cookie options @@ -96,8 +82,12 @@ class PhpSession extends Session } else { $options = PhpSession::$defaultCookieOptions; } + if (array_key_exists('test_session_name', $options)) { + $this->sessionName = $options['test_session_name']; + unset($options['test_session_name']); + } foreach ($options as $sessionVar => $value) { - if (ini_set("session.".$sessionVar, $value) === false) { + if (ini_set("session." . $sessionVar, $value) === false) { Logger::warn( 'Could not set php.ini setting %s = %s. This might affect your sessions behaviour.', $sessionVar, @@ -108,129 +98,38 @@ class PhpSession extends Session if (!is_writable(session_save_path())) { throw new ConfigurationError('Can\'t save session'); } + $this->read(); } /** - * Return true when the session has not yet been closed - * - * @return bool + * Open a PHP session */ - private function sessionCanBeChanged() + private function open() { - if ($this->isFlushed) { - Logger::error('Tried to work on a closed session, session changes will be ignored'); - return false; - } - return true; - } - - /** - * Return true when the session has not yet been opened - * - * @return bool - */ - private function sessionCanBeOpened() - { - if ($this->isOpen) { - Logger::warn('Tried to open a session more than once'); - return false; - } - return $this->sessionCanBeChanged(); - } - - /** - * Open a PHP session when possible - * - * @return bool True on success - */ - public function open() - { - if (!$this->sessionCanBeOpened()) { - return false; - } - - session_name(self::SESSION_NAME); + session_name($this->sessionName); session_start(); - $this->isOpen = true; + } + + /** + * Read all values written to the underling session and make them accessible. + */ + public function read() + { + $this->open(); $this->setAll($_SESSION); - return true; - } - - /** - * Ensure that the session is open modifiable - * - * @return bool True on success - */ - private function ensureOpen() - { - // try to open first - if (!$this->isOpen) { - if (!$this->open()) { - return false; - } - } - return true; - } - - /** - * Read all values written to the underling session and - * makes them accessible. if keepOpen is not set, the session - * is immediately closed again - * - * @param bool $keepOpen Set to true when modifying the session - * - * @return bool True on success - */ - public function read($keepOpen = false) - { - if (!$this->ensureOpen()) { - return false; - } - if ($keepOpen) { - return true; - } - $this->close(); - return true; + session_write_close(); } /** * Write all values of this session object to the underlying session implementation - * - * If keepOpen is not set, the session is closed - * - * @param bool $keepOpen Set to true when modifying the session further - * - * @return bool True on success */ - public function write($keepOpen = false) + public function write() { - if (!$this->ensureOpen()) { - return false; - } + $this->open(); foreach ($this->getAll() as $key => $value) { $_SESSION[$key] = $value; } - if ($keepOpen) { - return null; - } - $this->close(); - - return null; - } - - /** - * Close and writes the session - * - * Only call this if you want the session to be closed without any changes. - * - * @see PHPSession::write - */ - public function close() - { - if (!$this->isFlushed) { - session_write_close(); - } - $this->isFlushed = true; + session_write_close(); } /** @@ -238,12 +137,12 @@ class PhpSession extends Session */ public function purge() { - if ($this->ensureOpen()) { - $_SESSION = array(); - session_destroy(); - $this->clearCookies(); - $this->close(); - } + $this->open(); + $_SESSION = array(); + $this->setAll(array(), true); + session_destroy(); + $this->clearCookies(); + session_write_close(); } /** diff --git a/library/Icinga/Authentication/Session.php b/library/Icinga/Authentication/Session.php index 0cdd5b3bd..ad2a65f5e 100644 --- a/library/Icinga/Authentication/Session.php +++ b/library/Icinga/Authentication/Session.php @@ -41,29 +41,15 @@ abstract class Session */ private $sessionValues = array(); - /** - * Open a session or creates a new one if not exists - */ - abstract public function open(); - /** * Read all values from the underlying session implementation - * - * @param bool $keepOpen True to keep the session open */ - abstract public function read($keepOpen = false); + abstract public function read(); /** * Persists changes to the underlying session implementation - * - * @param bool $keepOpen True to keep the session open */ - abstract public function write($keepOpen = false); - - /** - * Close session - */ - abstract public function close(); + abstract public function write(); /** * Purge session diff --git a/library/Icinga/User.php b/library/Icinga/User.php index 9003e9f64..67e10e94f 100644 --- a/library/Icinga/User.php +++ b/library/Icinga/User.php @@ -31,7 +31,8 @@ namespace Icinga; use \DateTimeZone; use \InvalidArgumentException; -use Icinga\User\Preferences; +use \Icinga\User\Preferences; +use \Icinga\Authentication\PhpSession; /** * This class represents an authorized user @@ -105,6 +106,13 @@ class User */ private $preferences; + /** + * Queued notifications for this user. + * + * @var array() + */ + private $messages; + /** * Creates a user object given the provided information * @@ -334,4 +342,19 @@ class User } return new DateTimeZone($tz); } + + /** + * Send a message to this user that can be accessed in other requests. + * + * @param $msg + */ + public function sendMessage($msg) + { + $this->messages[] = $msg; + } + + public function getMessages() + { + return $this->messages; + } } diff --git a/library/Icinga/Web/Controller/ActionController.php b/library/Icinga/Web/Controller/ActionController.php index b7e2ee826..b153e7411 100755 --- a/library/Icinga/Web/Controller/ActionController.php +++ b/library/Icinga/Web/Controller/ActionController.php @@ -58,17 +58,6 @@ class ActionController extends Zend_Controller_Action */ protected $requiresAuthentication = true; - /** - * Set true when this controller modifies the session - * - * otherwise the session will be written back to disk and closed before the controller - * action is executed, leading to every modification in the session to be lost after - * the response is submitted - * - * @var bool - */ - protected $modifiesSession = false; - /** * The constructor starts benchmarking, loads the configuration and sets * other useful controller properties @@ -114,10 +103,7 @@ class ActionController extends Zend_Controller_Action return false; } - return !AuthManager::getInstance( - null, - array('writeSession' => $this->modifiesSession) - )->isAuthenticated(); + return !AuthManager::getInstance()->isAuthenticated(); } /** diff --git a/test/php/library/Icinga/Authentication/PhpSessionTest.php b/test/php/library/Icinga/Authentication/PhpSessionTest.php index 793293980..cf7e2fee9 100644 --- a/test/php/library/Icinga/Authentication/PhpSessionTest.php +++ b/test/php/library/Icinga/Authentication/PhpSessionTest.php @@ -49,67 +49,54 @@ class PhpSessionTest extends BaseTestCase { private function getSession() { - if (!is_writable('/tmp')) { $this->markTestSkipped('Could not write to session directory'); } return new PhpSession( array( 'use_cookies' => false, - 'save_path' => '/tmp' + 'save_path' => '/tmp', + 'test_session_name' => 'IcingawebUnittest' ) ); - } /** - * Test the creation of a PhpSession object - * - * @runInSeparateProcess - **/ + * Test the creation of a PhpSession object + * + * @runInSeparateProcess + */ public function testSessionCreation() { $this->getSession(); } /** - * Test PhpSession::open() + * Test PhpSession::open() * - * @runInSeparateProcess + * @runInSeparateProcess */ - public function testOpenSession() + public function testSessionReadWrite() { - $this->assertEquals(session_id(), '', 'Asserting test precondition: session not being setup yet '); $session = $this->getSession(); - $session->open(); - $this->assertNotEquals(session_id(), '', 'Asserting a Session ID being available after PhpSession::open()'); + $session->purge(); + $this->assertEquals(null, $session->get('key')); + $session->set('key', 'value'); + $session->write(); + $session->read(); + $this->assertEquals('value', $session->get('key')); } /** - * Test a session being closed by PhpSession::close() + * Test a session being closed by PhpSession::close() * - * @runInSeparateProcess - **/ - public function testCloseSession() - { - $this->assertEquals(session_id(), '', 'Asserting test precondition: session not being setup yet '); - $session = $this->getSession(); - $session->open(); - $this->assertNotEquals(session_id(), '', 'Asserting a Session ID being available after PhpSession::open()'); - $session->close(); - } - - /** - * Test if a session is correctly purged when calling PhpSession::purge() - * - * @runInSeparateProcess + * @runInSeparateProcess */ public function testPurgeSession() { - $this->assertEquals(session_id(), '', 'Asserting test precondition: session not being setup yet '); $session = $this->getSession(); - $session->open(); - $this->assertNotEquals(session_id(), '', 'Asserting a Session ID being available after PhpSession::open()'); + $session->set('key2', 'value2'); $session->purge(); - $this->assertEquals(session_id(), '', 'Asserting no Session ID being available after PhpSession::purge()'); + $session->read(); + $this->assertEquals(null, $session->get('key2')); } } From 2a0add3ec3c94d667b03452305456a2605da20ce Mon Sep 17 00:00:00 2001 From: Matthias Jentsch Date: Wed, 20 Nov 2013 19:10:38 +0100 Subject: [PATCH 2/3] Fix messages in ConfigController Store messages in the current user session to be able to fetch messages from other controllers, so that the use can be redirected back to the index, instead of staying in the original action refs #5100 --- application/controllers/ConfigController.php | 142 ++++++++++++------ .../Config/Resource/EditResourceForm.php | 4 + .../views/scripts/config/authentication.phtml | 17 +-- .../config/authentication/create.phtml | 9 +- .../config/authentication/modify.phtml | 9 +- .../config/authentication/remove.phtml | 5 + application/views/scripts/config/index.phtml | 9 +- .../views/scripts/config/logging.phtml | 4 + .../scripts/config/module/overview.phtml | 9 +- .../views/scripts/config/resource.phtml | 23 +-- .../scripts/config/resource/create.phtml | 10 +- .../scripts/config/resource/modify.phtml | 11 +- .../scripts/config/resource/remove.phtml | 7 +- .../scripts/config/show-configuration.phtml | 2 + application/views/scripts/error/error.phtml | 27 +--- library/Icinga/Authentication/Manager.php | 5 +- library/Icinga/Authentication/Session.php | 3 + library/Icinga/User.php | 28 +++- library/Icinga/User/Message.php | 57 +++++++ .../Web/Controller/BaseConfigController.php | 49 +++++- library/Icinga/Web/Widget/AlertMessageBox.php | 113 ++++++++++++++ .../controllers/ConfigController.php | 24 +-- .../controllers/ShowController.php | 1 - .../views/scripts/config/editbackend.phtml | 6 +- .../views/scripts/config/editinstance.phtml | 5 +- .../views/scripts/config/index.phtml | 21 ++- .../views/scripts/config/removebackend.phtml | 4 +- .../views/scripts/config/removeinstance.phtml | 6 +- .../scripts/config/show-configuration.phtml | 2 +- .../Icinga/Authentication/ManagerTest.php | 11 -- 30 files changed, 432 insertions(+), 191 deletions(-) create mode 100644 library/Icinga/User/Message.php create mode 100644 library/Icinga/Web/Widget/AlertMessageBox.php diff --git a/application/controllers/ConfigController.php b/application/controllers/ConfigController.php index 93b655090..72b98e31a 100644 --- a/application/controllers/ConfigController.php +++ b/application/controllers/ConfigController.php @@ -30,13 +30,15 @@ use \Icinga\Web\Controller\BaseConfigController; use \Icinga\Web\Widget\Tab; +use \Icinga\Web\Widget\AlertMessageBox; use \Icinga\Web\Url; -use \Icinga\Web\Widget\Tabs; use \Icinga\Web\Hook\Configuration\ConfigurationTabBuilder; +use \Icinga\User\Message; use \Icinga\Application\Icinga; use \Icinga\Application\Config as IcingaConfig; use \Icinga\Data\ResourceFactory; use \Icinga\Form\Config\GeneralForm; +use \Icinga\Authentication\Manager as AuthenticationManager; use \Icinga\Form\Config\Authentication\ReorderForm; use \Icinga\Form\Config\Authentication\LdapBackendForm; use \Icinga\Form\Config\Authentication\DbBackendForm; @@ -57,6 +59,7 @@ class ConfigController extends BaseConfigController */ private $resourceTypes = array('livestatus', 'ido', 'statusdat', 'ldap'); + /** * Create tabs for this configuration controller * @@ -110,17 +113,18 @@ class ConfigController extends BaseConfigController */ public function indexAction() { - $form = new GeneralForm(); + $this->view->messageBox = new AlertMessageBox($this->getAndClearMessages()); + $form = new GeneralForm(); $form->setConfiguration(IcingaConfig::app()); $form->setRequest($this->_request); if ($form->isSubmittedAndValid()) { if (!$this->writeConfigFile($form->getConfig(), 'config')) { return; } - $this->view->successMessage = "Config Sucessfully Updated"; + $this->addSuccessMessage("Configuration Sucessfully Updated"); $form->setConfiguration(IcingaConfig::app(), true); - + $this->redirectNow('config/index'); } $this->view->form = $form; } @@ -130,6 +134,8 @@ class ConfigController extends BaseConfigController */ public function loggingAction() { + $this->view->messageBox = new AlertMessageBox($this->getAndClearMessages()); + $form = new LoggingForm(); $form->setConfiguration(IcingaConfig::app()); $form->setRequest($this->_request); @@ -137,8 +143,9 @@ class ConfigController extends BaseConfigController if (!$this->writeConfigFile($form->getConfig(), 'config')) { return; } - $this->view->successMessage = "Config Sucessfully Updated"; + $this->addSuccessMessage("Configuration Sucessfully Updated"); $form->setConfiguration(IcingaConfig::app(), true); + $this->redirectNow('config/logging'); } $this->view->form = $form; } @@ -148,6 +155,9 @@ class ConfigController extends BaseConfigController */ public function moduleoverviewAction() { + $this->view->messageBox = new AlertMessageBox($this->getAndClearMessages()); + + $this->view->messages = $this->getAndClearMessages(); $this->view->modules = Icinga::app()->getModuleManager()->select() ->from('modules') ->order('name'); @@ -164,10 +174,11 @@ class ConfigController extends BaseConfigController try { $manager->enableModule($module); $manager->loadModule($module); - $this->view->successMessage = 'Module "' . $module . '" enabled'; - $this->moduleoverviewAction(); + $this->addSuccessMessage('Module "' . $module . '" enabled'); + $this->redirectNow('config/moduleoverview'); + return; } catch (Exception $e) { - $this->view->exceptionMessage = $e->getMessage(); + $this->view->exceptionMesssage = $e->getMessage(); $this->view->moduleName = $module; $this->view->action = 'enable'; $this->render('module-configuration-error'); @@ -183,8 +194,9 @@ class ConfigController extends BaseConfigController $manager = Icinga::app()->getModuleManager(); try { $manager->disableModule($module); - $this->view->successMessage = 'Module "' . $module . '" disabled'; - $this->moduleoverviewAction(); + $this->addSuccessMessage('Module "' . $module . '" disabled'); + $this->redirectNow('config/moduleoverview'); + return; } catch (Exception $e) { $this->view->exceptionMessage = $e->getMessage(); $this->view->moduleName = $module; @@ -201,6 +213,7 @@ class ConfigController extends BaseConfigController $config = IcingaConfig::app('authentication', true); $order = array_keys($config->toArray()); $this->view->backends = array(); + $this->view->messageBox = new AlertMessageBox($this->getAndClearMessages()); foreach ($config as $backend=>$backendConfig) { $form = new ReorderForm(); @@ -211,8 +224,8 @@ class ConfigController extends BaseConfigController if (!$showOnly && $form->isSubmittedAndValid()) { if ($this->writeAuthenticationFile($form->getReorderedConfig($config))) { - $this->view->successMessage = 'Authentication Order Updated'; - $this->authenticationAction(true); + $this->addSuccessMessage('Authentication Order Updated'); + $this->redirectNow('config/authentication'); } return; } @@ -230,6 +243,8 @@ class ConfigController extends BaseConfigController */ public function createauthenticationbackendAction() { + $this->view->messageBox = new AlertMessageBox(); + if ($this->getRequest()->getParam('type') === 'ldap') { $form = new LdapBackendForm(); } else { @@ -248,7 +263,7 @@ class ConfigController extends BaseConfigController foreach ($form->getConfig() as $backendName => $settings) { unset($settings->{'name'}); if (isset($backendCfg[$backendName])) { - $this->view->errorMessage = 'Backend name already exists'; + $this->view->messageBox->addError('Backend name already exists'); $this->view->form = $form; $this->render('authentication/create'); return; @@ -257,11 +272,13 @@ class ConfigController extends BaseConfigController } if ($this->writeAuthenticationFile($backendCfg)) { // redirect to overview with success message - $this->view->successMessage = 'Backend Modification Written'; - $this->authenticationAction(true); + $this->addSuccessMessage('Backend Modification Written.'); + $this->redirectNow("config/authentication"); } return; } + + $this->view->messageBox->addForm($form); $this->view->form = $form; $this->render('authentication/create'); } @@ -278,13 +295,13 @@ class ConfigController extends BaseConfigController $configArray = IcingaConfig::app('authentication', true)->toArray(); $authBackend = $this->getParam('auth_backend'); if (!isset($configArray[$authBackend])) { - $this->view->errorMessage = 'Can\'t edit: Unknown Authentication Backend Provided'; - $this->authenticationAction(true); + $this->addErrorMessage('Can\'t edit: Unknown Authentication Backend Provided'); + $this->configurationerrorAction(); return; } if (!array_key_exists('resource', $configArray[$authBackend])) { - $this->view->errorMessage = 'Configuration error: Backend "' . $authBackend . '" has no Resource'; - $this->authenticationAction(true); + $this->addErrorMessage('Configuration error: Backend "' . $authBackend . '" has no Resource'); + $this->configurationerrorAction(); return; } @@ -297,8 +314,8 @@ class ConfigController extends BaseConfigController $form = new DbBackendForm(); break; default: - $this->view->errorMessage = 'Can\'t edit: backend type "' . $type . '" of given resource not supported.'; - $this->authenticationAction(true); + $this->addErrorMessage('Can\'t edit: backend type "' . $type . '" of given resource not supported.'); + $this->configurationerrorAction(); return; } @@ -318,12 +335,13 @@ class ConfigController extends BaseConfigController } if ($this->writeAuthenticationFile($backendCfg)) { // redirect to overview with success message - $this->view->successMessage = 'Backend "' . $authBackend . '" created'; - $this->authenticationAction(true); + $this->addSuccessMessage('Backend "' . $authBackend . '" created'); + $this->redirectNow("config/authentication"); } return; } + $this->view->messageBox->addForm($form); $this->view->name = $authBackend; $this->view->form = $form; $this->render('authentication/modify'); @@ -336,16 +354,22 @@ class ConfigController extends BaseConfigController */ public function removeauthenticationbackendAction() { + $this->view->messageBox = new AlertMessageBox(); + $configArray = IcingaConfig::app('authentication', true)->toArray(); $authBackend = $this->getParam('auth_backend'); if (!isset($configArray[$authBackend])) { - $this->view->errorMessage = 'Can\'t perform removal: Unknown Authentication Backend Provided'; - $this->authenticationAction(true); + $this->view->messageBox->addMessage( + new Message('Can\'t perform removal: Unknown Authentication Backend Provided', Zend_Log::ERR) + ); + $this->render('authentication/remove'); return; } if (!array_key_exists('resource', $configArray[$authBackend])) { - $this->view->errorMessage = 'Configuration error: Backend "' . $authBackend . '" has no Resource'; - $this->authenticationAction(true); + $this->view->messageBox->addMessage( + new Message('Configuration error: Backend "' . $authBackend . '" has no Resource', Zend_Log::ERR) + ); + $this->render('authentication/remove'); return; } @@ -356,8 +380,8 @@ class ConfigController extends BaseConfigController if ($form->isSubmittedAndValid()) { unset($configArray[$authBackend]); if ($this->writeAuthenticationFile($configArray)) { - $this->view->successMessage = 'Authentication Backend "' . $authBackend . '" Removed'; - $this->authenticationAction(true); + $this->addSuccessMessage('Authentication Backend "' . $authBackend . '" Removed'); + $this->redirectNow("config/authentication"); } return; } @@ -369,6 +393,7 @@ class ConfigController extends BaseConfigController public function resourceAction($showOnly = false) { + $this->view->messageBox = new AlertMessageBox($this->getAndClearMessages()); $this->view->resources = IcingaConfig::app('resources', true)->toArray(); $this->render('resource'); @@ -383,29 +408,33 @@ class ConfigController extends BaseConfigController if ($form->isSubmittedAndValid()) { $name = $form->getName(); if (isset($resources->{$name})) { - $this->view->errorMessage = 'Resource name "' . $name .'" already in use.'; - $this->view->form = $form; - $this->render('resource/create'); - return; + $this->addErrorMessage('Resource name "' . $name .'" already in use.'); + } else { + $resources->{$name} = $form->getConfig(); + if ($this->writeConfigFile($resources, 'resources')) { + $this->addSuccessMessage('Resource "' . $name . '" created.'); + $this->redirectNow("config/resource"); + } } - $resources->{$name} = $form->getConfig(); - if ($this->writeConfigFile($resources, 'resources')) { - $this->view->successMessage = 'Resource "' . $name . '" created.'; - $this->resourceAction(true); - } - return; } + + $this->view->messageBox = new AlertMessageBox($this->getAndClearMessages()); + $this->view->messageBox->addForm($form); $this->view->form = $form; $this->render('resource/create'); } public function editresourceAction() { + $this->view->messageBox = new AlertMessageBox(); + $resources = ResourceFactory::getResourceConfigs(); $name = $this->getParam('resource'); if ($resources->get($name) === null) { - $this->view->errorMessage = 'Can\'t edit: Unknown Resource Provided'; - $this->resourceAction(true); + $this->view->messageBox->addMessage( + new Message('Can\'t edit: Unknown Resource Provided', Zend_Log::ERR) + ); + $this->render('resource/modify'); return; } $form = new EditResourceForm(); @@ -423,11 +452,13 @@ class ConfigController extends BaseConfigController } $resources->{$name} = $form->getConfig(); if ($this->writeConfigFile($resources, 'resources')) { - $this->view->successMessage = 'Resource "' . $name . '" created.'; - $this->resourceAction(true); + $this->addSuccessMessage('Resource "' . $name . '" edited.'); + $this->redirectNow("config/resource"); } return; } + + $this->view->messageBox->addForm($form); $this->view->form = $form; $this->view->name = $name; $this->render('resource/modify'); @@ -435,11 +466,15 @@ class ConfigController extends BaseConfigController public function removeresourceAction() { + $this->view->messageBox = new AlertMessageBox(); + $resources = ResourceFactory::getResourceConfigs()->toArray(); $name = $this->getParam('resource'); if (!isset($resources[$name])) { - $this->view->errorMessage = 'Can\'t remove: Unknown resource provided'; - $this->resourceAction(true); + $this->view->messageBox->addMessage( + new Message('Can\'t remove: Unknown resource provided', Zend_Log::ERR) + ); + $this->render('resource/remove'); return; } @@ -449,8 +484,8 @@ class ConfigController extends BaseConfigController if ($form->isSubmittedAndValid()) { unset($resources[$name]); if ($this->writeConfigFile($resources, 'resources')) { - $this->view->successMessage = 'Resource "' . $name . '" removed'; - $this->resourceAction(true); + $this->addSuccessMessage('Resource "' . $name . '" removed.'); + $this->redirectNow('config/resource'); } return; } @@ -460,6 +495,19 @@ class ConfigController extends BaseConfigController $this->render('resource/remove'); } + + /** + * Redirect target only for error-states + * + * When an error is opened in the side-pane, redirecting this request to the index or the overview will look + * weird. This action returns a clear page containing only an AlertMessageBox. + */ + public function configurationerrorAction() + { + $this->view->messageBox = new AlertMessageBox($this->getAndClearMessages()); + $this->render('error/error'); + } + /** * Write changes to an authentication file * diff --git a/application/forms/Config/Resource/EditResourceForm.php b/application/forms/Config/Resource/EditResourceForm.php index 800720097..a70263efe 100644 --- a/application/forms/Config/Resource/EditResourceForm.php +++ b/application/forms/Config/Resource/EditResourceForm.php @@ -390,6 +390,9 @@ class EditResourceForm extends Form !file_exists($config->object_file) || !file_exists($config->status_file) ) { + $this->addErrorMessage( + 'Connectivity validation failed, the provided file or socket does not exist.' + ); return false; } break; @@ -402,6 +405,7 @@ class EditResourceForm extends Form break; } } catch (\Exception $exc) { + $this->addErrorMessage('Connectivity validation failed, connection to the given resource not possible.'); return false; } return true; diff --git a/application/views/scripts/config/authentication.phtml b/application/views/scripts/config/authentication.phtml index 4bb8d947d..ab96e4f13 100644 --- a/application/views/scripts/config/authentication.phtml +++ b/application/views/scripts/config/authentication.phtml @@ -6,20 +6,9 @@ $createDbBackend = $this->href('/config/createAuthenticationBackend', array('typ tabs->render($this); ?> -errorMessage): ?> -
- - escape($this->errorMessage); ?> -
- - - -successMessage): ?> -
- - escape($this->successMessage); ?> -
- +messageBox)): ?> + messageBox->render() ?> +
diff --git a/application/views/scripts/config/authentication/create.phtml b/application/views/scripts/config/authentication/create.phtml index 69e0b4bd3..fd329b63a 100644 --- a/application/views/scripts/config/authentication/create.phtml +++ b/application/views/scripts/config/authentication/create.phtml @@ -3,11 +3,10 @@ Create New Authentication Backend -errorMessage): ?> -
- escape($this->errorMessage); ?> -
- + +messageBox)): ?> + messageBox->render() ?> +

Create a new backend for authenticating your users. This backend will be added at the end of your authentication order. diff --git a/application/views/scripts/config/authentication/modify.phtml b/application/views/scripts/config/authentication/modify.phtml index 298cbdd6e..ce7ca8a0a 100644 --- a/application/views/scripts/config/authentication/modify.phtml +++ b/application/views/scripts/config/authentication/modify.phtml @@ -3,11 +3,14 @@ Edit Backend "escape($this->name); ?>" -errorMessage || $this->form->getErrorMessages()): ?> +messageBox)): ?> + messageBox->render() ?> + + +form->getErrorMessages()): ?>

- escape($this->errorMessage); ?> form->getErrorMessages() as $error): ?> - escape($error); ?>
+ escape($error); ?>
diff --git a/application/views/scripts/config/authentication/remove.phtml b/application/views/scripts/config/authentication/remove.phtml index 9187195d5..ed478d24b 100644 --- a/application/views/scripts/config/authentication/remove.phtml +++ b/application/views/scripts/config/authentication/remove.phtml @@ -2,4 +2,9 @@ Remove Backend "escape($this->name); ?>" + +messageBox)): ?> + messageBox->render() ?> + + form ?> \ No newline at end of file diff --git a/application/views/scripts/config/index.phtml b/application/views/scripts/config/index.phtml index 98e227f7d..d307c73eb 100644 --- a/application/views/scripts/config/index.phtml +++ b/application/views/scripts/config/index.phtml @@ -1,10 +1,7 @@ tabs->render($this); ?> -successMessage): ?> -
- - escape($this->successMessage); ?> -
- +messageBox)): ?> + messageBox->render() ?> + form ?> \ No newline at end of file diff --git a/application/views/scripts/config/logging.phtml b/application/views/scripts/config/logging.phtml index 6b0b7cca3..d8129ff62 100644 --- a/application/views/scripts/config/logging.phtml +++ b/application/views/scripts/config/logging.phtml @@ -2,6 +2,10 @@ form->getErrorMessages(); ?> +messageBox)): ?> + messageBox->render() ?> + + successMessage): ?>
diff --git a/application/views/scripts/config/module/overview.phtml b/application/views/scripts/config/module/overview.phtml index e7330c9c3..f50b2e30d 100644 --- a/application/views/scripts/config/module/overview.phtml +++ b/application/views/scripts/config/module/overview.phtml @@ -9,12 +9,9 @@ $modules = $this->modules->paginate();

Installed Modules

-successMessage): ?> -
- - escape($this->successMessage); ?> -
- +messageBox)): ?> + messageBox->render() ?> + paginationControl($modules, null, null, array( 'preserve' => $this->preserve diff --git a/application/views/scripts/config/resource.phtml b/application/views/scripts/config/resource.phtml index 4c167aeef..cb6f9542c 100644 --- a/application/views/scripts/config/resource.phtml +++ b/application/views/scripts/config/resource.phtml @@ -2,28 +2,11 @@ use Icinga\Web\Url; $createResource = $this->href('/config/createresource'); ?> - tabs->render($this); ?> -errorMessage): ?> -
- - escape($this->errorMessage); ?> -
- - -successMessage): ?> -
- - escape($this->successMessage); ?> -
-flashMessages): ?> -
- - escape($this->flashMessages); ?> -
- - +messageBox)): ?> + messageBox->render() ?> +
diff --git a/application/views/scripts/config/resource/create.phtml b/application/views/scripts/config/resource/create.phtml index 6ad1bcb4c..f82354557 100644 --- a/application/views/scripts/config/resource/create.phtml +++ b/application/views/scripts/config/resource/create.phtml @@ -3,14 +3,12 @@ Create New Resource -errorMessage): ?> -
- escape($this->errorMessage); ?> -
- +messageBox)): ?> + messageBox->render() ?> +

- Create a new resource to describes a data sourc + Resources are entities that provide data to Icingaweb.

form ?> \ No newline at end of file diff --git a/application/views/scripts/config/resource/modify.phtml b/application/views/scripts/config/resource/modify.phtml index 9956c3cdf..079c9464d 100644 --- a/application/views/scripts/config/resource/modify.phtml +++ b/application/views/scripts/config/resource/modify.phtml @@ -3,13 +3,16 @@ Edit Resource "escape($this->name); ?>" -errorMessage || $this->form->getErrorMessages()): ?> +messageBox)): ?> + messageBox->render() ?> + + +form->getErrorMessages()): ?>
- escape($this->errorMessage); ?> form->getErrorMessages() as $error): ?> - escape($error); ?>
+ escape($error); ?>
- + form ?> \ No newline at end of file diff --git a/application/views/scripts/config/resource/remove.phtml b/application/views/scripts/config/resource/remove.phtml index 0502cc709..3b5ffbfc1 100644 --- a/application/views/scripts/config/resource/remove.phtml +++ b/application/views/scripts/config/resource/remove.phtml @@ -2,4 +2,9 @@ Remove Resource "escape($this->name); ?>" -form ?> \ No newline at end of file + +messageBox)): ?> + messageBox->render() ?> + + +form ?> diff --git a/application/views/scripts/config/show-configuration.phtml b/application/views/scripts/config/show-configuration.phtml index 79d8edf59..2bf97cbfb 100644 --- a/application/views/scripts/config/show-configuration.phtml +++ b/application/views/scripts/config/show-configuration.phtml @@ -1,4 +1,6 @@ tabs->render($this); ?> + +

WARNING ICONSaving "escape($this->file); ?>.ini" Failed

diff --git a/application/views/scripts/error/error.phtml b/application/views/scripts/error/error.phtml index f9c73e026..c51ab59da 100755 --- a/application/views/scripts/error/error.phtml +++ b/application/views/scripts/error/error.phtml @@ -1,26 +1,9 @@ +tabs->render($this); ?> +

An error occurred

-

- message ?> -

- exception)): ?> -
-

Exception information:

- -

- Message: - exception->getMessage(); ?> -

- -

Stack trace:

- 
-      exception->getTraceAsString(); ?>
-
- -

Request Parameters:

- 
getParams(), true); ?>
-
-
- + messageBox)) : ?> + messageBox->render() ?> +
diff --git a/library/Icinga/Authentication/Manager.php b/library/Icinga/Authentication/Manager.php index 9445581f7..83c9393e3 100644 --- a/library/Icinga/Authentication/Manager.php +++ b/library/Icinga/Authentication/Manager.php @@ -347,7 +347,6 @@ class Manager return false; } - $this->user->sendMessage("Hallo ich bin nachricht."); if ($persist == true) { $this->persistCurrentUser(); $this->session->write(); @@ -399,9 +398,9 @@ class Manager } /** - * Returns the current user or null if no user is authenticated + * Returns the current user or null if no user is authenticated * - * @return User + * @return User **/ public function getUser() { diff --git a/library/Icinga/Authentication/Session.php b/library/Icinga/Authentication/Session.php index ad2a65f5e..8959549a6 100644 --- a/library/Icinga/Authentication/Session.php +++ b/library/Icinga/Authentication/Session.php @@ -64,10 +64,13 @@ abstract class Session * @see self::persist * @param string $key Name of value * @param mixed $value Value + * + * @returns PhpSession this */ public function set($key, $value) { $this->sessionValues[$key] = $value; + return $this; } /** diff --git a/library/Icinga/User.php b/library/Icinga/User.php index 67e10e94f..4a35ac721 100644 --- a/library/Icinga/User.php +++ b/library/Icinga/User.php @@ -32,14 +32,15 @@ namespace Icinga; use \DateTimeZone; use \InvalidArgumentException; use \Icinga\User\Preferences; +use \Icinga\User\Message; use \Icinga\Authentication\PhpSession; + /** * This class represents an authorized user * * You can retrieve authorization information (@TODO: Not implemented yet) or * to retrieve user information - * */ class User { @@ -344,17 +345,34 @@ class User } /** - * Send a message to this user that can be accessed in other requests. + * Add a message that can be accessed from future requests, to this user. * - * @param $msg + * This function does NOT automatically write to the session, messages will not be persisted until you do. + * + * @param Message $msg The message */ - public function sendMessage($msg) + public function addMessage(Message $msg) { $this->messages[] = $msg; } + /** + * Get all currently pending messages + * + * @return array the messages + */ public function getMessages() { - return $this->messages; + return isset($this->messages) ? $this->messages : array(); + } + + /** + * Remove all messages from this user + * + * This function does NOT automatically write the session, messages will not be persisted until you do. + */ + public function clearMessages() + { + $this->messages = null; } } diff --git a/library/Icinga/User/Message.php b/library/Icinga/User/Message.php new file mode 100644 index 000000000..d10bb09f5 --- /dev/null +++ b/library/Icinga/User/Message.php @@ -0,0 +1,57 @@ +message = $message; + $this->level = $level; + } + + /** + * @return string + */ + public function getMessage() + { + return $this->message; + } + + /** + * @return The + */ + public function getLevel() + { + return $this->level; + } +} \ No newline at end of file diff --git a/library/Icinga/Web/Controller/BaseConfigController.php b/library/Icinga/Web/Controller/BaseConfigController.php index 86e108cbc..aca2dc758 100644 --- a/library/Icinga/Web/Controller/BaseConfigController.php +++ b/library/Icinga/Web/Controller/BaseConfigController.php @@ -30,6 +30,9 @@ namespace Icinga\Web\Controller; use \Icinga\Application\Icinga; +use \Icinga\Authentication\Manager as AuthenticationManager; +use \Zend_Log; +use \Icinga\User\Message; /** * Base class for Configuration Controllers @@ -38,7 +41,6 @@ use \Icinga\Application\Icinga; * added to the application's configuration dialog. If you create a subclass of * BasePreferenceController and overwrite @see init(), make sure you call * parent::init(), otherwise you won't have the $tabs property in your view. - * */ class BaseConfigController extends ActionController { @@ -47,6 +49,50 @@ class BaseConfigController extends ActionController */ protected $flashManager; + /** + * Remove all messages from the current user, return them and commit + * changes to the underlying session. + * + * @return array The messages + */ + protected function getAndClearMessages() + { + // empty all messages + $user = AuthenticationManager::getInstance()->getUser(); + $messages = $user->getMessages(); + $user->clearMessages(); + AuthenticationManager::getInstance()->getSession()->write(); + return $messages; + } + + /** + * Send a message with the logging level Zend_Log::INFO to the current user and + * commit the changes to the underlying session. + * + * @param $msg The message content + */ + protected function addSuccessMessage($msg) + { + AuthenticationManager::getInstance()->getUser()->addMessage( + new Message($msg, Zend_Log::INFO) + ); + AuthenticationManager::getInstance()->getSession()->write(); + } + + /** + * Send a message with the logging level Zend_Log::ERR to the current user and + * commit the changes to the underlying session. + * + * @param $msg The message content + */ + protected function addErrorMessage($msg) + { + AuthenticationManager::getInstance()->getUser()->addMessage( + new Message($msg, Zend_Log::ERR) + ); + AuthenticationManager::getInstance()->getSession()->write(); + } + /* * Return an array of tabs provided by this configuration controller. * @@ -68,6 +114,5 @@ class BaseConfigController extends ActionController { parent::init(); $this->view->tabs = ControllerTabCollector::collectControllerTabs('ConfigController'); - $this->view->flashMessages = $this->_request->getParam('flash_message'); } } diff --git a/library/Icinga/Web/Widget/AlertMessageBox.php b/library/Icinga/Web/Widget/AlertMessageBox.php new file mode 100644 index 000000000..ccb7bc2f1 --- /dev/null +++ b/library/Icinga/Web/Widget/AlertMessageBox.php @@ -0,0 +1,113 @@ + array( + 'state' => 'alert-success', + 'icon' => 'icinga-icon-success' + ), + Zend_Log::NOTICE => array( + 'state' => 'alert-info', + 'icon' => 'icinga-icon-info' + ), + Zend_Log::WARN => array( + 'state' => 'alert-warning', + 'icon' => 'icinga-icon-warning' + ), + Zend_Log::ERR => array( + 'state' => 'alert-danger', + 'icon' => 'icinga-icon-danger' + ) + ); + + /** + * Create a new AlertBox + * + * @param Message|array $messages The message(s) to display + */ + public function __construct($messages = array()) { + if (!is_array($messages)) { + $this->messages = array($messages); + } else { + $this->messages = $messages; + } + } + + /** + * Add a new message. + * + * @param Message $message + */ + public function addMessage(Message $message) + { + $this->messages[] = $message; + } + + /** + * Add a new error + * + * @param $error + */ + public function addError($error) + { + $this->messages[] = new Message($error, Zend_Log::ERR); + } + + /** + * Add the error messages of the given Zend_Form + */ + public function addForm(Zend_Form $form) + { + foreach ($form->getErrorMessages() as $error) { + $this->addError($error); + } + } + + /** + * Output the HTML of the AlertBox + * + * @return string + */ + public function render(Zend_View_Abstract $view = null) { + $html = ''; + foreach ($this->messages as $message) { + $level = $message->getLevel(); + if (!array_key_exists($level, $this->states)) { + continue; + } + $alert = $this->states[$level]; + $html .= '
' . + '' . + '' . htmlspecialchars($message->getMessage()) . '' . + '
'; + } + return $html; + } +} \ No newline at end of file diff --git a/modules/monitoring/application/controllers/ConfigController.php b/modules/monitoring/application/controllers/ConfigController.php index 755a06173..045390eaf 100644 --- a/modules/monitoring/application/controllers/ConfigController.php +++ b/modules/monitoring/application/controllers/ConfigController.php @@ -34,6 +34,7 @@ use \Icinga\Application\Config as IcingaConfig; use \Icinga\Config\PreservingIniWriter; use \Icinga\Web\Controller\BaseConfigController; use \Icinga\Web\Widget\Tab; +use \Icinga\Web\Widget\AlertMessageBox; use \Icinga\Web\Url; use Icinga\Module\Monitoring\Form\Config\ConfirmRemovalForm; @@ -69,6 +70,7 @@ class Monitoring_ConfigController extends BaseConfigController { */ public function indexAction() { + $this->view->messageBox = new AlertMessageBox($this->getAndClearMessages()); $this->view->backends = IcingaConfig::module('monitoring', 'backends')->toArray(); $this->view->instances = IcingaConfig::module('monitoring', 'instances')->toArray(); $this->render('index'); @@ -89,11 +91,11 @@ class Monitoring_ConfigController extends BaseConfigController { $backendForm->setBackendConfiguration(IcingaConfig::module('monitoring', 'backends')->get($backend)); if ($backendForm->isSubmittedAndValid()) { - $newConfig = $backendForm->getConfig(); $config = IcingaConfig::module('monitoring', 'backends'); $config->$backend = $newConfig; if ($this->writeConfiguration($config, 'backends')) { + $this->addSuccessMessage('Backend ' . $backend . ' Modified.'); $this->redirectNow('monitoring/config'); } else { $this->render('show-configuration'); @@ -116,8 +118,8 @@ class Monitoring_ConfigController extends BaseConfigController { $configArray[$form->getBackendName()] = $form->getConfig(); if ($this->writeConfiguration(new Zend_Config($configArray), 'backends')) { - $this->view->successMessage = 'Backend Creation Succeeded'; - $this->indexAction(); + $this->addSuccessMessage('Backend Creation Succeeded'); + $this->redirectNow('monitoring/config'); } else { $this->render('show-configuration'); } @@ -146,8 +148,8 @@ class Monitoring_ConfigController extends BaseConfigController { unset($configArray[$backend]); if ($this->writeConfiguration(new Zend_Config($configArray), 'backends')) { - $this->view->successMessage = 'Backend "' . $backend . '" Removed'; - $this->indexAction(); + $this->addSuccessMessage('Backend "' . $backend . '" Removed'); + $this->redirectNow('monitoring/config'); } else { $this->render('show-configuration'); } @@ -178,8 +180,8 @@ class Monitoring_ConfigController extends BaseConfigController { unset($configArray[$instance]); if ($this->writeConfiguration(new Zend_Config($configArray), 'instances')) { - $this->view->successMessage = 'Instance "' . $instance . '" Removed'; - $this->indexAction(); + $this->addSuccessMessage('Instance "' . $instance . '" Removed'); + $this->redirectNow('monitoring/config'); } else { $this->render('show-configuration'); } @@ -207,8 +209,8 @@ class Monitoring_ConfigController extends BaseConfigController { $instanceConfig = IcingaConfig::module('monitoring', 'instances')->toArray(); $instanceConfig[$instance] = $form->getConfig(); if ($this->writeConfiguration(new Zend_Config($instanceConfig), 'instances')) { - $this->view->successMessage = 'Instance Modified'; - $this->indexAction(); + $this->addSuccessMessage('Instance Modified'); + $this->redirectNow('monitoring/config'); } else { $this->render('show-configuration'); return; @@ -228,8 +230,8 @@ class Monitoring_ConfigController extends BaseConfigController { $instanceConfig = IcingaConfig::module('monitoring', 'instances')->toArray(); $instanceConfig[$form->getInstanceName()] = $form->getConfig()->toArray(); if ($this->writeConfiguration(new Zend_Config($instanceConfig), 'instances')) { - $this->view->successMessage = 'Instance Creation Succeeded'; - $this->indexAction(); + $this->addSuccessMessage('Instance Creation Succeeded'); + $this->redirectNow('monitoring/config'); } else { $this->render('show-configuration'); } diff --git a/modules/monitoring/application/controllers/ShowController.php b/modules/monitoring/application/controllers/ShowController.php index 569500a82..39febc428 100644 --- a/modules/monitoring/application/controllers/ShowController.php +++ b/modules/monitoring/application/controllers/ShowController.php @@ -56,7 +56,6 @@ class Monitoring_ShowController extends MonitoringController */ public function init() { - if ($this->getRequest()->getActionName() === 'host') { $this->view->object = new Host($this->getRequest()); } elseif ($this->getRequest()->getActionName() === 'service' diff --git a/modules/monitoring/application/views/scripts/config/editbackend.phtml b/modules/monitoring/application/views/scripts/config/editbackend.phtml index 4344f9dbe..7ecf69bf0 100644 --- a/modules/monitoring/application/views/scripts/config/editbackend.phtml +++ b/modules/monitoring/application/views/scripts/config/editbackend.phtml @@ -1,8 +1,8 @@ -tabs->render($this); ?> + name): ?> -

{{EDIT_ICON}} Edit Backend "escape($this->name) ?>"

+

Edit Backend "escape($this->name) ?>"

-

{{CREATE_ICON}} Create New Backend

+

Create New Backend

error): ?> diff --git a/modules/monitoring/application/views/scripts/config/editinstance.phtml b/modules/monitoring/application/views/scripts/config/editinstance.phtml index 5b6907c64..1a2f5fe97 100644 --- a/modules/monitoring/application/views/scripts/config/editinstance.phtml +++ b/modules/monitoring/application/views/scripts/config/editinstance.phtml @@ -1,9 +1,8 @@ -tabs->render($this); ?> name)): ?> -

{{EDIT_ICON}} Edit Instance Configuration for "escape($this->name) ?>"

+

} Edit Instance Configuration for "escape($this->name) ?>"

-

{{CREATE_ICON}} Configure New Icinga Instance

+

Configure New Icinga Instance

error): ?> diff --git a/modules/monitoring/application/views/scripts/config/index.phtml b/modules/monitoring/application/views/scripts/config/index.phtml index a12501f9f..f9e1f0ea8 100644 --- a/modules/monitoring/application/views/scripts/config/index.phtml +++ b/modules/monitoring/application/views/scripts/config/index.phtml @@ -3,16 +3,13 @@

Monitoring Backends

-successMessage): ?> -
- {{OK_ICON}} - escape($this->successMessage); ?> -
- +messageBox)): ?> + messageBox->render() ?> +
@@ -24,8 +21,8 @@ escape($backendName); ?> (Type: escape($config['type'] === 'ido' ? 'IDO' : ucfirst($config['type'])); ?>)

@@ -37,7 +34,7 @@ @@ -50,8 +47,8 @@ (Type: )

diff --git a/modules/monitoring/application/views/scripts/config/removebackend.phtml b/modules/monitoring/application/views/scripts/config/removebackend.phtml index e2a3e0226..bd4a95f3b 100644 --- a/modules/monitoring/application/views/scripts/config/removebackend.phtml +++ b/modules/monitoring/application/views/scripts/config/removebackend.phtml @@ -1,5 +1,5 @@ -tabs->render($this); ?> -

{{REMOVE_ICON}} Remove Backend "escape($this->name) ?>"

+ +

Remove Backend "escape($this->name) ?>"

Are you sure you want to remove the backend escape($this->name) ?>? diff --git a/modules/monitoring/application/views/scripts/config/removeinstance.phtml b/modules/monitoring/application/views/scripts/config/removeinstance.phtml index 6a9711a44..a278cf9b1 100644 --- a/modules/monitoring/application/views/scripts/config/removeinstance.phtml +++ b/modules/monitoring/application/views/scripts/config/removeinstance.phtml @@ -1,12 +1,12 @@ -tabs->render($this); ?> -

{{REMOVE_ICON}} Remove Instance "escape($this->name) ?>"

+ +

Remove Instance "escape($this->name) ?>"

Are you sure you want to remove the instance escape($this->name) ?>?

- {{WARNING_ICON}} If you have still any environments or views refering to this instance, you won't be able to send commands anymore + If you have still any environments or views refering to this instance, you won't be able to send commands anymore after deletion.

diff --git a/modules/monitoring/application/views/scripts/config/show-configuration.phtml b/modules/monitoring/application/views/scripts/config/show-configuration.phtml index 884a46aac..356c42b7b 100644 --- a/modules/monitoring/application/views/scripts/config/show-configuration.phtml +++ b/modules/monitoring/application/views/scripts/config/show-configuration.phtml @@ -1,7 +1,7 @@ tabs->render($this); ?>
-

{{ERROR_ICON}} Saving escape($this->file); ?>failed

+

Saving escape($this->file); ?>failed


Your escape($this->file); ?> configuration couldn't be stored (error: "exceptionMessage; ?>").
diff --git a/test/php/library/Icinga/Authentication/ManagerTest.php b/test/php/library/Icinga/Authentication/ManagerTest.php index 73fc430d4..18e58ff9f 100644 --- a/test/php/library/Icinga/Authentication/ManagerTest.php +++ b/test/php/library/Icinga/Authentication/ManagerTest.php @@ -179,17 +179,6 @@ class ManagerTest extends BaseTestCase $this->assertTrue($auth->isAuthenticated()); } - /** - * @expectedException Exception - * @expectedExceptionMessage Session write after close - */ - public function testWriteSessionTwice() - { - $auth = $this->getManagerInstance($session, false); - $this->assertFalse($auth->isAuthenticated(true)); - $auth->authenticate(new Credential("jdoe", "passjdoe")); - } - /** * @expectedException Icinga\Exception\ConfigurationError * @expectedExceptionMessage No authentication backend set From cd0194e20f7a978dc239a4d485208c6d00f2a2ec Mon Sep 17 00:00:00 2001 From: Matthias Jentsch Date: Tue, 26 Nov 2013 10:35:46 +0100 Subject: [PATCH 3/3] Fetch and clean user messages lazily, to ensure that only shown messages are removed This will make the code of the ConfigController way easier, as messages can be send from everywhere and there is no need to consider consquences of redirections. refs #5100 --- application/controllers/ConfigController.php | 62 +++++++++---------- .../Web/Controller/BaseConfigController.php | 21 ------- library/Icinga/Web/Widget/AlertMessageBox.php | 57 ++++++++++------- .../controllers/ConfigController.php | 2 +- 4 files changed, 66 insertions(+), 76 deletions(-) diff --git a/application/controllers/ConfigController.php b/application/controllers/ConfigController.php index 72b98e31a..edee923fa 100644 --- a/application/controllers/ConfigController.php +++ b/application/controllers/ConfigController.php @@ -5,7 +5,6 @@ * This file is part of Icinga Web 2. * * Icinga Web 2 - Head for multiple monitoring backends. - * Copyright (C) 2013 Icinga Development Team * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -59,7 +58,6 @@ class ConfigController extends BaseConfigController */ private $resourceTypes = array('livestatus', 'ido', 'statusdat', 'ldap'); - /** * Create tabs for this configuration controller * @@ -113,8 +111,7 @@ class ConfigController extends BaseConfigController */ public function indexAction() { - $this->view->messageBox = new AlertMessageBox($this->getAndClearMessages()); - + $this->view->messageBox = new AlertMessageBox(true); $form = new GeneralForm(); $form->setConfiguration(IcingaConfig::app()); $form->setRequest($this->_request); @@ -134,7 +131,7 @@ class ConfigController extends BaseConfigController */ public function loggingAction() { - $this->view->messageBox = new AlertMessageBox($this->getAndClearMessages()); + $this->view->messageBox = new AlertMessageBox(true); $form = new LoggingForm(); $form->setConfiguration(IcingaConfig::app()); @@ -155,9 +152,8 @@ class ConfigController extends BaseConfigController */ public function moduleoverviewAction() { - $this->view->messageBox = new AlertMessageBox($this->getAndClearMessages()); + $this->view->messageBox = new AlertMessageBox(true); - $this->view->messages = $this->getAndClearMessages(); $this->view->modules = Icinga::app()->getModuleManager()->select() ->from('modules') ->order('name'); @@ -213,7 +209,7 @@ class ConfigController extends BaseConfigController $config = IcingaConfig::app('authentication', true); $order = array_keys($config->toArray()); $this->view->backends = array(); - $this->view->messageBox = new AlertMessageBox($this->getAndClearMessages()); + $this->view->messageBox = new AlertMessageBox(true); foreach ($config as $backend=>$backendConfig) { $form = new ReorderForm(); @@ -243,7 +239,7 @@ class ConfigController extends BaseConfigController */ public function createauthenticationbackendAction() { - $this->view->messageBox = new AlertMessageBox(); + $this->view->messageBox = new AlertMessageBox(true); if ($this->getRequest()->getParam('type') === 'ldap') { $form = new LdapBackendForm(); @@ -263,7 +259,7 @@ class ConfigController extends BaseConfigController foreach ($form->getConfig() as $backendName => $settings) { unset($settings->{'name'}); if (isset($backendCfg[$backendName])) { - $this->view->messageBox->addError('Backend name already exists'); + $this->addErrorMessage('Backend name already exists'); $this->view->form = $form; $this->render('authentication/create'); return; @@ -292,6 +288,8 @@ class ConfigController extends BaseConfigController */ public function editauthenticationbackendAction() { + $this->view->messageBox = new AlertMessageBox(true); + $configArray = IcingaConfig::app('authentication', true)->toArray(); $authBackend = $this->getParam('auth_backend'); if (!isset($configArray[$authBackend])) { @@ -354,21 +352,12 @@ class ConfigController extends BaseConfigController */ public function removeauthenticationbackendAction() { - $this->view->messageBox = new AlertMessageBox(); + $this->view->messageBox = new AlertMessageBox(true); $configArray = IcingaConfig::app('authentication', true)->toArray(); $authBackend = $this->getParam('auth_backend'); if (!isset($configArray[$authBackend])) { - $this->view->messageBox->addMessage( - new Message('Can\'t perform removal: Unknown Authentication Backend Provided', Zend_Log::ERR) - ); - $this->render('authentication/remove'); - return; - } - if (!array_key_exists('resource', $configArray[$authBackend])) { - $this->view->messageBox->addMessage( - new Message('Configuration error: Backend "' . $authBackend . '" has no Resource', Zend_Log::ERR) - ); + $this->addSuccessMessage('Can\'t perform removal: Unknown Authentication Backend Provided'); $this->render('authentication/remove'); return; } @@ -393,8 +382,7 @@ class ConfigController extends BaseConfigController public function resourceAction($showOnly = false) { - $this->view->messageBox = new AlertMessageBox($this->getAndClearMessages()); - + $this->view->messageBox = new AlertMessageBox(true); $this->view->resources = IcingaConfig::app('resources', true)->toArray(); $this->render('resource'); } @@ -418,7 +406,7 @@ class ConfigController extends BaseConfigController } } - $this->view->messageBox = new AlertMessageBox($this->getAndClearMessages()); + $this->view->messageBox = new AlertMessageBox(true); $this->view->messageBox->addForm($form); $this->view->form = $form; $this->render('resource/create'); @@ -426,14 +414,12 @@ class ConfigController extends BaseConfigController public function editresourceAction() { - $this->view->messageBox = new AlertMessageBox(); + $this->view->messageBox = new AlertMessageBox(true); $resources = ResourceFactory::getResourceConfigs(); $name = $this->getParam('resource'); if ($resources->get($name) === null) { - $this->view->messageBox->addMessage( - new Message('Can\'t edit: Unknown Resource Provided', Zend_Log::ERR) - ); + $this->addErrorMessage('Can\'t edit: Unknown Resource Provided'); $this->render('resource/modify'); return; } @@ -466,14 +452,12 @@ class ConfigController extends BaseConfigController public function removeresourceAction() { - $this->view->messageBox = new AlertMessageBox(); + $this->view->messageBox = new AlertMessageBox(true); $resources = ResourceFactory::getResourceConfigs()->toArray(); $name = $this->getParam('resource'); if (!isset($resources[$name])) { - $this->view->messageBox->addMessage( - new Message('Can\'t remove: Unknown resource provided', Zend_Log::ERR) - ); + $this->addSuccessMessage('Can\'t remove: Unknown resource provided'); $this->render('resource/remove'); return; } @@ -481,6 +465,18 @@ class ConfigController extends BaseConfigController $form = new ConfirmRemovalForm(); $form->setRequest($this->getRequest()); $form->setRemoveTarget('resource', $name); + + // Check if selected resource is currently used for authentication + $authConfig = IcingaConfig::app('authentication', true)->toArray(); + foreach ($authConfig as $backendName => $config) { + if (array_key_exists('resource', $config) && $config['resource'] === $name) { + $this->addErrorMessage( + 'Warning: The resource "' . $name . '" is currently used for user authentication by "' . $backendName . '". ' . + ' Deleting it could eventally make login impossible.' + ); + } + } + if ($form->isSubmittedAndValid()) { unset($resources[$name]); if ($this->writeConfigFile($resources, 'resources')) { @@ -504,7 +500,7 @@ class ConfigController extends BaseConfigController */ public function configurationerrorAction() { - $this->view->messageBox = new AlertMessageBox($this->getAndClearMessages()); + $this->view->messageBox = new AlertMessageBox(true); $this->render('error/error'); } diff --git a/library/Icinga/Web/Controller/BaseConfigController.php b/library/Icinga/Web/Controller/BaseConfigController.php index aca2dc758..5ca93e9aa 100644 --- a/library/Icinga/Web/Controller/BaseConfigController.php +++ b/library/Icinga/Web/Controller/BaseConfigController.php @@ -44,27 +44,6 @@ use \Icinga\User\Message; */ class BaseConfigController extends ActionController { - /** - * @var Zend_Controller_Action_Helper_FlashMessenger - */ - protected $flashManager; - - /** - * Remove all messages from the current user, return them and commit - * changes to the underlying session. - * - * @return array The messages - */ - protected function getAndClearMessages() - { - // empty all messages - $user = AuthenticationManager::getInstance()->getUser(); - $messages = $user->getMessages(); - $user->clearMessages(); - AuthenticationManager::getInstance()->getSession()->write(); - return $messages; - } - /** * Send a message with the logging level Zend_Log::INFO to the current user and * commit the changes to the underlying session. diff --git a/library/Icinga/Web/Widget/AlertMessageBox.php b/library/Icinga/Web/Widget/AlertMessageBox.php index ccb7bc2f1..9bf90facf 100644 --- a/library/Icinga/Web/Widget/AlertMessageBox.php +++ b/library/Icinga/Web/Widget/AlertMessageBox.php @@ -4,24 +4,47 @@ namespace Icinga\Web\Widget; use \Zend_Log; use \Zend_Form; +use \Icinga\User; use \Icinga\User\Message; use \Zend_View_Abstract; +use \Icinga\Authentication\Manager as AuthenticationManager; /** - * Class AlertMessageBox + * Displays a set of alert messages to the user. * - * Displays a set of alert messages to the user. - * - * @package Icinga\Web\Widget + * The messages are fetched automatically from the current AuthenticationManager, + * but this is done lazily when render() is called, to ensure that messages will + * always be displayed before they are cleared. */ class AlertMessageBox implements \Icinga\Web\Widget\Widget { + /** + * Remove all messages from the current user, return them and commit + * changes to the underlying session. + * + * @return array The messages + */ + protected function getAndClearMessages() + { + $messages = $this->user->getMessages(); + $this->user->clearMessages(); + AuthenticationManager::getInstance()->getSession()->write(); + return $messages; + } + /** * The displayed alert messages * * @var array */ private $messages = array(); + + /** + * The user to fetch the messages from + * + * @var User + */ + private $user; /** * The available states. @@ -50,26 +73,15 @@ class AlertMessageBox implements \Icinga\Web\Widget\Widget { /** * Create a new AlertBox * - * @param Message|array $messages The message(s) to display + * @param boolean showUserMessages If the current user messages should be displayed + * in this AlertMessageBox. Defaults to false */ - public function __construct($messages = array()) { - if (!is_array($messages)) { - $this->messages = array($messages); - } else { - $this->messages = $messages; + public function __construct($showUserMessages = false) { + if ($showUserMessages) { + $this->user = AuthenticationManager::getInstance()->getUser(); } } - /** - * Add a new message. - * - * @param Message $message - */ - public function addMessage(Message $message) - { - $this->messages[] = $message; - } - /** * Add a new error * @@ -97,6 +109,9 @@ class AlertMessageBox implements \Icinga\Web\Widget\Widget { */ public function render(Zend_View_Abstract $view = null) { $html = ''; + if (isset($this->user)) { + $this->messages = array_merge($this->messages, $this->getAndClearMessages()); + } foreach ($this->messages as $message) { $level = $message->getLevel(); if (!array_key_exists($level, $this->states)) { @@ -110,4 +125,4 @@ class AlertMessageBox implements \Icinga\Web\Widget\Widget { } return $html; } -} \ No newline at end of file +} diff --git a/modules/monitoring/application/controllers/ConfigController.php b/modules/monitoring/application/controllers/ConfigController.php index 045390eaf..c75a54829 100644 --- a/modules/monitoring/application/controllers/ConfigController.php +++ b/modules/monitoring/application/controllers/ConfigController.php @@ -70,7 +70,7 @@ class Monitoring_ConfigController extends BaseConfigController { */ public function indexAction() { - $this->view->messageBox = new AlertMessageBox($this->getAndClearMessages()); + $this->view->messageBox = new AlertMessageBox(true); $this->view->backends = IcingaConfig::module('monitoring', 'backends')->toArray(); $this->view->instances = IcingaConfig::module('monitoring', 'instances')->toArray(); $this->render('index');