Skip to content

Commit 0f0d0b5

Browse files
nilmergnilmerg
authored
Streamline authentication process (#5387)
This is split into four distinct parts: (One commit is only performing a clean up) ### 1. EmbeddedWeb: Explicitly perform authentication It is nowadays no exception that stylesheet may be dependent on who's using the app. So to avoid race conditions like in #5385 authentication is an explicit step during bootstrap now. `EmbeddedWeb` now knows about and is responsible to initially set up the user object. So this does not change when `Web` sets up the user, it just allows `EmbeddedWeb` to do it. A small side-effect introduced by this change is fully optional, but doesn't break anything either: Method `Web::setupSession()` and the accompanying `Web::$session` property are gone now as the property was obsolete already. Initializing the session is still done at the previous stage though, but implicitly by `Web::setupNotifications()`. ### 2. Auth: Perform authentication only once and not lazily Since authentication is now performed even for static resources, there's no reason anymore to support implicit authentication. This also limits authentication attempts to a single one, previously failed attempts were repeated. Requiring authentication during bootstrapping, i.e. before authentication has been performed, will now throw an error. This conflicts with #5430 as it also affects `Auth::isAuthenticated()`. But this should be easy to resolve. ### 3. Manager: Perform module loading asynchronously So that authentication can suspend it. There are cases, e.g. cube, where authentication is required in run.php. During bootstrapping loading modules is mostly required to load libraries, register routes and hooks. Most of the time authentication is not required for these, but if it is, evaluation is now interrupted and continued after authentication has actually been performed. I don't see a real risk for any breaking change here, since authentication happens shortly after. It actually avoids a breaking change, since without this, cube's Icinga DB support would break or at least malfunction. And cube is only a single example. As long as the *monitoring* module is still supported by them, they will keep being affected. Enable the *monitoring* module and Icinga DB Web in an environment and look at the debug log to see this change in effect. ### 4. Module: Expect user and group backends upon registration Providing a user or user group backend in configuration.php now triggers a deprecation or warning. They are expected to be announced in run.php, just like hooks. Authentication attempts during a configuration.php run, while Authentication has not been performed yet, will be also suspended now. This ensures that the current behavior, being broken or working, stays the same. You might think that this is a breaking change, but it was rather always broken and only worked in such particular situations that custom backends were never used in conjunction with any of the official modules mentioned in 3. Removing the compatibility layer (the suspension part) with v2.15 might indeed be a bit hasty. But I rather like to get rid of this as early as possible and I think those who have custom extensions like this, in a functional state, are a minority and known to us. (i.e. Icinga partners which we can forewarn)
2 parents feeb03d + 0306f2b commit 0f0d0b5

6 files changed

Lines changed: 211 additions & 103 deletions

File tree

‎doc/80-Upgrading.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ v2.6 to v2.8 requires to follow the instructions for v2.7 too.
55

66
## Upgrading to Icinga Web 2.14
77

8+
**Deprecations**
9+
10+
* Providing a custom user or group backend inside a module's configuration.php is deprecated and won't work as of v2.15.
11+
* It continues to work under the same circumstances as before and remains broken where it was already broken.
12+
* To keep it working as of v2.15 or make it work now, provide the custom backend inside the module's run.php instead.
13+
814
**Framework changes affecting third-party code**
915

1016
* Our JavaScript implementation to update relative times in the UI has been removed from Icinga Web, and an updated

‎library/Icinga/Application/EmbeddedWeb.php‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
require_once dirname(__FILE__) . '/ApplicationBootstrap.php';
99

10+
use Icinga\Authentication\Auth;
11+
use Icinga\User;
1012
use Icinga\Web\Request;
1113
use Icinga\Web\Response;
1214
use ipl\I18n\NoopTranslator;
@@ -37,6 +39,13 @@ class EmbeddedWeb extends ApplicationBootstrap
3739
*/
3840
protected $response;
3941

42+
/**
43+
* User object
44+
*
45+
* @var ?User
46+
*/
47+
protected ?User $user = null;
48+
4049
/**
4150
* Get the request
4251
*
@@ -67,17 +76,19 @@ public function getResponse()
6776
protected function bootstrap()
6877
{
6978
return $this
79+
->setupLogging()
7080
->setupErrorHandling()
7181
->loadLibraries()
7282
->loadConfig()
73-
->setupLogging()
7483
->setupLogger()
7584
->setupRequest()
7685
->setupResponse()
7786
->setupTimezone()
7887
->prepareFakeInternationalization()
7988
->setupModuleManager()
8089
->loadEnabledModules()
90+
->setupUserBackendFactory()
91+
->setupUser()
8192
->registerApplicationHooks();
8293
}
8394

@@ -103,6 +114,22 @@ protected function setupResponse()
103114
return $this;
104115
}
105116

117+
/**
118+
* Create user object
119+
*
120+
* @return $this
121+
*/
122+
protected function setupUser(): static
123+
{
124+
$auth = Auth::getInstance();
125+
if ($auth->authenticate()->isAuthenticated()) {
126+
$this->user = $auth->getUser();
127+
$this->getRequest()->setUser($this->user);
128+
}
129+
130+
return $this;
131+
}
132+
106133
/**
107134
* Prepare fake internationalization
108135
*

‎library/Icinga/Application/Modules/Manager.php‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
namespace Icinga\Application\Modules;
77

8+
use Fiber;
89
use Icinga\Application\ApplicationBootstrap;
910
use Icinga\Application\Icinga;
1011
use Icinga\Application\Logger;
@@ -189,8 +190,23 @@ private function detectEnabledModules()
189190
public function loadEnabledModules()
190191
{
191192
if (! $this->loadedAllEnabledModules) {
193+
$async = ! $this->app->isCli();
192194
foreach ($this->listEnabledModules() as $name) {
193-
$this->loadModule($name);
195+
if ($async) {
196+
// May be suspended during authentication and resumed upon finish
197+
(new Fiber(function () use ($name) {
198+
Logger::debug(
199+
'Loading enabled module "%s" asynchronously (Process %d; Fiber %d)',
200+
$name,
201+
getmypid() ?: 0,
202+
spl_object_id(Fiber::getCurrent())
203+
);
204+
205+
$this->loadModule($name);
206+
}))->start();
207+
} else {
208+
$this->loadModule($name);
209+
}
194210
}
195211

196212
$this->loadedAllEnabledModules = true;

‎library/Icinga/Application/Modules/Module.php‎

Lines changed: 63 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
namespace Icinga\Application\Modules;
77

88
use Exception;
9+
use Fiber;
910
use Icinga\Application\ApplicationBootstrap;
1011
use Icinga\Application\Config;
1112
use Icinga\Application\Hook;
@@ -20,7 +21,6 @@
2021
use ipl\I18n\GettextTranslator;
2122
use ipl\I18n\StaticTranslator;
2223
use ipl\I18n\Translation;
23-
use Zend_Controller_Router_Route;
2424
use Zend_Controller_Router_Route_Abstract;
2525
use Zend_Controller_Router_Route_Regex;
2626

@@ -1085,7 +1085,22 @@ public function getSetupWizard()
10851085
*/
10861086
public function getUserBackends()
10871087
{
1088-
$this->launchConfigScript();
1088+
// TODO: Remove with v2.15
1089+
if (Fiber::getCurrent() === null) {
1090+
(new Fiber(function () {
1091+
Logger::debug(
1092+
'Running config script of module "%s" asynchronously (Process %d; Fiber %d)',
1093+
$this->getName(),
1094+
getmypid() ?: 0,
1095+
spl_object_id(Fiber::getCurrent())
1096+
);
1097+
1098+
$this->launchConfigScript();
1099+
}))->start();
1100+
} else {
1101+
$this->launchConfigScript();
1102+
}
1103+
10891104
return $this->userBackends;
10901105
}
10911106

@@ -1096,7 +1111,22 @@ public function getUserBackends()
10961111
*/
10971112
public function getUserGroupBackends()
10981113
{
1099-
$this->launchConfigScript();
1114+
// TODO: Remove with v2.15
1115+
if (Fiber::getCurrent() === null) {
1116+
(new Fiber(function () {
1117+
Logger::debug(
1118+
'Running config script of module "%s" asynchronously (Process %d; Fiber %d)',
1119+
$this->getName(),
1120+
getmypid() ?: 0,
1121+
spl_object_id(Fiber::getCurrent())
1122+
);
1123+
1124+
$this->launchConfigScript();
1125+
}))->start();
1126+
} else {
1127+
$this->launchConfigScript();
1128+
}
1129+
11001130
return $this->userGroupBackends;
11011131
}
11021132

@@ -1197,6 +1227,21 @@ protected function provideSetupWizard($className)
11971227
*/
11981228
protected function provideUserBackend($identifier, $className)
11991229
{
1230+
if ($this->registered) {
1231+
trigger_error(sprintf(
1232+
'Module "%s" already registered. Providing user backend "%s" in configuration.php'
1233+
. ' has no effect as of version 2.15. Put it in run.php instead.',
1234+
$this->getName(),
1235+
$identifier
1236+
), E_USER_DEPRECATED);
1237+
Logger::warning(
1238+
'Module "%s" already registered. Providing user backend "%s" in configuration.php'
1239+
. ' has no effect as of version 2.15. Put it in run.php instead.',
1240+
$this->getName(),
1241+
$identifier
1242+
);
1243+
}
1244+
12001245
$this->userBackends[strtolower($identifier)] = $className;
12011246
return $this;
12021247
}
@@ -1211,6 +1256,21 @@ protected function provideUserBackend($identifier, $className)
12111256
*/
12121257
protected function provideUserGroupBackend($identifier, $className)
12131258
{
1259+
if ($this->registered) {
1260+
trigger_error(sprintf(
1261+
'Module "%s" already registered. Providing user group backend "%s" in configuration.php'
1262+
. ' has no effect as of version 2.15. Put it in run.php instead.',
1263+
$this->getName(),
1264+
$identifier
1265+
), E_USER_DEPRECATED);
1266+
Logger::warning(
1267+
'Module "%s" already registered. Providing user group backend "%s" in configuration.php'
1268+
. ' has no effect as of version 2.15. Put it in run.php instead.',
1269+
$this->getName(),
1270+
$identifier
1271+
);
1272+
}
1273+
12141274
$this->userGroupBackends[strtolower($identifier)] = $className;
12151275
return $this;
12161276
}
@@ -1341,18 +1401,6 @@ protected function registerRoutes()
13411401
foreach ($this->routes as $name => $route) {
13421402
$router->addRoute($name, $route);
13431403
}
1344-
$router->addRoute(
1345-
$this->name . '_jsprovider',
1346-
new Zend_Controller_Router_Route(
1347-
'js/' . $this->name . '/:file',
1348-
array(
1349-
'action' => 'javascript',
1350-
'controller' => 'static',
1351-
'module' => 'default',
1352-
'module_name' => $this->name
1353-
)
1354-
)
1355-
);
13561404
$router->addRoute(
13571405
$this->name . '_img',
13581406
new Zend_Controller_Router_Route_Regex(

‎library/Icinga/Application/Web.php‎

Lines changed: 14 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,15 @@
1313
use ipl\I18n\StaticTranslator;
1414
use Zend_Controller_Action_HelperBroker;
1515
use Zend_Controller_Front;
16-
use Zend_Controller_Router_Route;
1716
use Zend_Layout;
1817
use Zend_Paginator;
1918
use Zend_View_Helper_PaginationControl;
2019
use Icinga\Authentication\Auth;
21-
use Icinga\User;
2220
use Icinga\Util\DirectoryIterator;
2321
use Icinga\Util\TimezoneDetect;
2422
use Icinga\Web\Controller\Dispatcher;
2523
use Icinga\Web\Navigation\Navigation;
2624
use Icinga\Web\Notification;
27-
use Icinga\Web\Session;
28-
use Icinga\Web\Session\Session as BaseSession;
2925
use Icinga\Web\StyleSheet;
3026
use Icinga\Web\View;
3127

@@ -54,20 +50,6 @@ class Web extends EmbeddedWeb
5450
*/
5551
private $frontController;
5652

57-
/**
58-
* Session object
59-
*
60-
* @var BaseSession
61-
*/
62-
private $session;
63-
64-
/**
65-
* User object
66-
*
67-
* @var User
68-
*/
69-
private $user;
70-
7153
/** @var array */
7254
protected $accessibleMenuItems;
7355

@@ -92,15 +74,13 @@ protected function bootstrap()
9274
->loadConfig()
9375
->setupLogger()
9476
->setupRequest()
95-
->setupSession()
9677
->setupNotifications()
9778
->setupResponse()
9879
->setupZendMvc()
9980
->prepareInternationalization()
10081
->setupModuleManager()
10182
->loadSetupModuleIfNecessary()
10283
->loadEnabledModules()
103-
->setupRoute()
10484
->setupPagination()
10585
->setupUserBackendFactory()
10686
->setupUser()
@@ -137,27 +117,6 @@ public function getThemes()
137117
return array_combine($themes, $themes);
138118
}
139119

140-
/**
141-
* Prepare routing
142-
*
143-
* @return $this
144-
*/
145-
private function setupRoute()
146-
{
147-
$this->frontController->getRouter()->addRoute(
148-
'module_javascript',
149-
new Zend_Controller_Router_Route(
150-
'js/components/:module_name/:file',
151-
array(
152-
'controller' => 'static',
153-
'action' => 'javascript'
154-
)
155-
)
156-
);
157-
158-
return $this;
159-
}
160-
161120
/**
162121
* Getter for frontController
163122
*
@@ -315,48 +274,25 @@ private function setupZendMvc()
315274
return $this;
316275
}
317276

318-
/**
319-
* Create user object
320-
*
321-
* @return $this
322-
*/
323-
private function setupUser()
277+
protected function setupUser(): static
324278
{
325-
$auth = Auth::getInstance();
326-
if (! $this->request->isXmlHttpRequest() && $this->request->isApiRequest() && ! $auth->isAuthenticated()) {
327-
$auth->authHttp();
328-
}
329-
if ($auth->isAuthenticated()) {
330-
$user = $auth->getUser();
331-
$this->getRequest()->setUser($user);
332-
$this->user = $user;
333-
334-
if ($user->can('user/application/stacktraces')) {
335-
$displayExceptions = $this->user->getPreferences()->getValue(
336-
'icingaweb',
337-
'show_stacktraces'
338-
);
279+
parent::setupUser();
339280

340-
if ($displayExceptions !== null) {
341-
$this->frontController->setParams(
342-
array(
343-
'displayExceptions' => $displayExceptions
344-
)
345-
);
346-
}
281+
if ($this->user !== null && $this->user->can('user/application/stacktraces')) {
282+
$displayExceptions = $this->user->getPreferences()->getValue(
283+
'icingaweb',
284+
'show_stacktraces'
285+
);
286+
287+
if ($displayExceptions !== null) {
288+
$this->frontController->setParams(
289+
array(
290+
'displayExceptions' => $displayExceptions
291+
)
292+
);
347293
}
348294
}
349-
return $this;
350-
}
351295

352-
/**
353-
* Initialize a session provider
354-
*
355-
* @return $this
356-
*/
357-
private function setupSession()
358-
{
359-
$this->session = Session::create();
360296
return $this;
361297
}
362298

0 commit comments

Comments
 (0)