Skip to content

Commit 85f0fcc

Browse files
nuxsminnuxsmin
committed
* [MOD] Improved plugins data handling by encrypting the plugin's data
Signed-off-by: nuxsmin <nuxsmin@syspass.org>
1 parent 14bd6d5 commit 85f0fcc

File tree

7 files changed

+56
-80
lines changed

7 files changed

+56
-80
lines changed

‎src/lib/Controllers/AuthenticatorController.php

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -193,10 +193,13 @@ private function checkPin($pin, AuthenticatorData $authenticatorData)
193193
* @param AuthenticatorData $authenticatorData
194194
*
195195
* @return bool
196+
* @throws \Defuse\Crypto\Exception\CryptoException
196197
* @throws \Defuse\Crypto\Exception\EnvironmentIsBrokenException
197198
* @throws \SP\Core\Exceptions\ConstraintException
199+
* @throws \SP\Core\Exceptions\NoSuchPropertyException
198200
* @throws \SP\Core\Exceptions\QueryException
199201
* @throws \SP\Repositories\NoSuchItemException
202+
* @throws \SP\Services\ServiceException
200203
*/
201204
private function save2FAStatus(AuthenticatorData $authenticatorData)
202205
{
@@ -211,9 +214,7 @@ private function save2FAStatus(AuthenticatorData $authenticatorData)
211214
$authenticatorData->setDate(time());
212215
$authenticatorData->setRecoveryCodes($this->authenticatorService->generateRecoveryCodes());
213216

214-
$this->plugin->setDataForId($this->userData->getId(), $authenticatorData);
215-
216-
$this->authenticatorService->savePluginUserData($authenticatorData);
217+
$this->plugin->saveData($this->userData->getId(), $authenticatorData);
217218

218219
return $this->returnJsonResponse(
219220
JsonResponse::JSON_SUCCESS,
@@ -404,9 +405,10 @@ protected function initialize()
404405
$this->authenticatorService = $this->dic->get(AuthenticatorService::class);
405406
$this->pluginContext = $this->dic->get(PluginContext::class);
406407
$this->trackService = $this->dic->get(TrackService::class);
407-
$this->plugin = $this->dic->get(PluginManager::class)
408-
->getPlugin(Plugin::PLUGIN_NAME);
409408
$this->userData = $this->session->getUserData();
410409
$this->trackRequest = $this->trackService->getTrackRequest(__CLASS__);
410+
411+
$pluginManager = $this->dic->get(PluginManager::class);
412+
$this->plugin = $pluginManager->getPlugin(Plugin::PLUGIN_NAME, true);
411413
}
412414
}

‎src/lib/Controllers/AuthenticatorLoginController.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ final class AuthenticatorLoginController extends ControllerBase
5252
*
5353
* @throws \DI\DependencyException
5454
* @throws \DI\NotFoundException
55+
* @throws \SP\Core\Exceptions\SessionTimeout
5556
* @throws \SP\Services\Auth\AuthException
5657
*/
5758
public function indexAction()

‎src/lib/Controllers/PreferencesController.php

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
use Psr\Container\ContainerInterface;
2828
use SP\Core\Context\ContextInterface;
2929
use SP\Core\Events\Event;
30+
use SP\Modules\Web\Plugins\Authenticator\Models\AuthenticatorData;
3031
use SP\Modules\Web\Plugins\Authenticator\Services\AuthenticatorService;
3132
use SP\Modules\Web\Plugins\Authenticator\Util\PluginContext;
3233
use SP\Mvc\Controller\ExtensibleTabControllerInterface;
@@ -109,27 +110,29 @@ protected function getSecurityTab()
109110
// Datos del usuario de la sesión
110111
$userData = $this->context->getUserData();
111112

113+
/** @var AuthenticatorData $authenticatorData */
112114
$authenticatorData = $this->plugin->getData();
113115

114116
$qrCode = '';
115117

118+
if ($authenticatorData !== null) {
119+
$template->assign('chk2FAEnabled', $authenticatorData->isTwofaEnabled());
120+
$template->assign('expireDays', $authenticatorData->getExpireDays());
121+
} else {
122+
$authenticatorData = new AuthenticatorData();
123+
$template->assign('chk2FAEnabled', false);
124+
}
125+
126+
$this->pluginContext->setUserData($authenticatorData);
127+
116128
if (!$authenticatorData->isTwofaEnabled()) {
117129
$authenticatorData->setIV(AuthenticatorService::makeInitializationKey());
118130

119-
$this->pluginContext->setUserData($authenticatorData);
120-
121-
// $qrCode = $this->authenticatorService->getQrCodeFromUrl($userData->getLogin(), $authenticatorData->getIV());
122131
$qrCode = $this->authenticatorService->getQrCodeFromServer($userData->getLogin(), $authenticatorData->getIV());
123-
} elseif ($authenticatorData->isTwofaEnabled()
124-
&& $authenticatorData->getUserId() > 0
125-
) {
126-
$this->pluginContext->setUserData($authenticatorData);
127132
}
128133

129134
$template->assign('qrCode', $qrCode);
130135
$template->assign('userId', $userData->getId());
131-
$template->assign('chk2FAEnabled', $authenticatorData->isTwofaEnabled());
132-
$template->assign('expireDays', $authenticatorData->getExpireDays());
133136
$template->assign('route', 'authenticator/save');
134137
$template->assign('viewCodesRoute', 'authenticator/showRecoveryCodes');
135138
} catch (\Exception $e) {

‎src/lib/Plugin.php

Lines changed: 12 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
use SP\Mvc\Controller\ExtensibleTabControllerInterface;
3737
use SP\Plugin\PluginBase;
3838
use SP\Plugin\PluginOperation;
39-
use SP\Repositories\NoSuchItemException;
4039
use SplSubject;
4140

4241
/**
@@ -54,10 +53,6 @@ class Plugin extends PluginBase
5453
* @var ContainerInterface
5554
*/
5655
private $dic;
57-
/**
58-
* @var PluginOperation
59-
*/
60-
private $pluginOperation;
6156
/**
6257
* @var SessionContext
6358
*/
@@ -109,12 +104,12 @@ public function updateEvent($eventType, Event $event)
109104
{
110105
switch ($eventType) {
111106
case 'show.userSettings':
112-
/** @var ExtensibleTabControllerInterface $source */
113-
$source = $event->getSource(ExtensibleTabControllerInterface::class);
114-
115107
$this->loadData();
116-
(new PreferencesController($source, $this, $this->dic))
117-
->setUp();
108+
(new PreferencesController(
109+
$event->getSource(ExtensibleTabControllerInterface::class),
110+
$this,
111+
$this->dic)
112+
)->setUp();
118113
break;
119114
case 'login.finish':
120115
$this->loadData();
@@ -133,12 +128,8 @@ private function loadData()
133128
$this->session->getUserData()->getId(),
134129
AuthenticatorData::class
135130
);
136-
} catch (NoSuchItemException $e) {
137-
$this->data = new AuthenticatorData();
138131
} catch (\Exception $e) {
139132
processException($e);
140-
141-
$this->data = new AuthenticatorData();
142133
}
143134
}
144135

@@ -153,7 +144,9 @@ private function checkLogin(Event $event)
153144
{
154145
$pluginContext = $this->dic->get(PluginContext::class);
155146

156-
if ($this->data !== null && $this->data->isTwofaEnabled()) {
147+
if ($this->data !== null
148+
&& $this->data->isTwofaEnabled()
149+
) {
157150
$pluginContext->setTwoFApass(false);
158151
$this->session->setAuthCompleted(false);
159152

@@ -257,23 +250,6 @@ public function getName()
257250
return self::PLUGIN_NAME;
258251
}
259252

260-
/**
261-
* Establecer los datos de un Id
262-
*
263-
* @param $id
264-
* @param AuthenticatorData $authenticatorData
265-
*
266-
* @return Plugin
267-
* @throws \SP\Core\Exceptions\ConstraintException
268-
* @throws \SP\Core\Exceptions\QueryException
269-
*/
270-
public function setDataForId($id, AuthenticatorData $authenticatorData)
271-
{
272-
$this->pluginOperation->update($id, $authenticatorData);
273-
274-
return $this;
275-
}
276-
277253
/**
278254
* Eliminar los datos de un Id
279255
*
@@ -285,15 +261,15 @@ public function setDataForId($id, AuthenticatorData $authenticatorData)
285261
*/
286262
public function deleteDataForId($id)
287263
{
288-
$this->pluginOperation->delete($id);
264+
$this->pluginOperation->delete((int)$id);
289265
}
290266

291267
/**
292-
* @param mixed $pluginOperation
268+
* onLoad
293269
*/
294-
public function onLoad(PluginOperation $pluginOperation)
270+
public function onLoad()
295271
{
296-
$this->pluginOperation = $pluginOperation;
272+
$this->loadData();
297273
}
298274

299275
/**

‎src/lib/Services/AuthenticatorService.php

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -185,9 +185,11 @@ public function getQrCodeFromServer(string $login, string $iv)
185185
* @return string
186186
* @throws AuthenticatorException
187187
* @throws EnvironmentIsBrokenException
188+
* @throws \Defuse\Crypto\Exception\CryptoException
188189
* @throws \SP\Core\Exceptions\ConstraintException
190+
* @throws \SP\Core\Exceptions\NoSuchPropertyException
189191
* @throws \SP\Core\Exceptions\QueryException
190-
* @throws \SP\Repositories\NoSuchItemException
192+
* @throws \SP\Services\ServiceException
191193
*/
192194
public function pickRecoveryCode(AuthenticatorData $authenticatorData)
193195
{
@@ -220,34 +222,20 @@ public function pickRecoveryCode(AuthenticatorData $authenticatorData)
220222
}
221223

222224
/**
223-
* @param $codes
225+
* @param array $codes
224226
* @param AuthenticatorData $authenticatorData
225227
*
228+
* @throws \Defuse\Crypto\Exception\CryptoException
226229
* @throws \SP\Core\Exceptions\ConstraintException
230+
* @throws \SP\Core\Exceptions\NoSuchPropertyException
227231
* @throws \SP\Core\Exceptions\QueryException
228-
* @throws \SP\Repositories\NoSuchItemException
232+
* @throws \SP\Services\ServiceException
229233
*/
230234
private function saveRecoveryCodes(array $codes, AuthenticatorData $authenticatorData)
231235
{
232236
$authenticatorData->setRecoveryCodes($codes);
233237
$authenticatorData->setLastRecoveryTime(time());
234-
$this->savePluginUserData($authenticatorData);
235-
}
236-
237-
/**
238-
* Guardar datos del Plugin de un usuario
239-
*
240-
* @param AuthenticatorData $authenticatorData
241-
*
242-
* @return void
243-
* @throws \SP\Core\Exceptions\ConstraintException
244-
* @throws \SP\Core\Exceptions\QueryException
245-
* @throws \SP\Repositories\NoSuchItemException
246-
*/
247-
public function savePluginUserData(AuthenticatorData $authenticatorData)
248-
{
249-
$this->plugin->setDataForId($authenticatorData->getUserId(), $authenticatorData);
250-
$this->plugin->saveData();
238+
$this->plugin->saveData($authenticatorData->getUserId(), $authenticatorData);
251239
}
252240

253241
/**
@@ -272,7 +260,7 @@ public function generateRecoveryCodes()
272260
/**
273261
* Eliminar los datos del Plugin de un usuario
274262
*
275-
* @param $id
263+
* @param int $id
276264
*
277265
* @return void
278266
* @throws \SP\Core\Exceptions\ConstraintException
@@ -283,7 +271,6 @@ public function generateRecoveryCodes()
283271
public function deletePluginUserData($id)
284272
{
285273
$this->plugin->deleteDataForId($id);
286-
$this->plugin->saveData();
287274
}
288275

289276
/**
@@ -328,19 +315,26 @@ public function checkVersion()
328315
* @param string $code
329316
*
330317
* @return bool
318+
* @throws \Defuse\Crypto\Exception\CryptoException
331319
* @throws \SP\Core\Exceptions\ConstraintException
320+
* @throws \SP\Core\Exceptions\NoSuchPropertyException
332321
* @throws \SP\Core\Exceptions\QueryException
333-
* @throws \SP\Repositories\NoSuchItemException
322+
* @throws \SP\Services\ServiceException
334323
*/
335324
public function useRecoveryCode(AuthenticatorData $authenticatorData, $code)
336325
{
337326
$codes = $authenticatorData->getRecoveryCodes();
338327
$usedKey = array_search($code, $codes, true);
339328

340329
if ($usedKey !== false) {
341-
$this->saveRecoveryCodes(array_values(array_filter($codes, function ($key) use ($usedKey) {
342-
return $key !== $usedKey;
343-
}, ARRAY_FILTER_USE_KEY)), $authenticatorData);
330+
$this->saveRecoveryCodes(
331+
array_values(
332+
array_filter($codes,
333+
function ($key) use ($usedKey) {
334+
return $key !== $usedKey;
335+
}, ARRAY_FILTER_USE_KEY)
336+
),
337+
$authenticatorData);
344338

345339
return true;
346340
}
@@ -352,6 +346,6 @@ protected function initialize()
352346
{
353347
$this->extensionChecker = $this->dic->get(PhpExtensionChecker::class);
354348
$this->plugin = $this->dic->get(PluginManager::class)
355-
->getPluginInfo(Plugin::PLUGIN_NAME);
349+
->getPlugin(Plugin::PLUGIN_NAME);
356350
}
357351
}

‎src/themes/material-blue/views/userpreferences/preferences-security.inc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ $chk2FAEnabled = $_getvar('chk2FAEnabled');
4040
<label class="mdl-switch mdl-js-switch mdl-js-ripple-effect" for="2faenabled">
4141
<input type="checkbox" id="2faenabled"
4242
class="mdl-switch__input mdl-color-text--indigo-400"
43-
name="2faenabled" <?php echo $_getvar('chk2FAEnabled') ? 'checked' : ''; ?>/>
43+
name="2faenabled" <?php echo $chk2FAEnabled ? 'checked' : ''; ?>/>
4444
<span class="mdl-switch__label"><?php echo _t('authenticator', 'Enable'); ?></span>
4545
</label>
4646

‎version.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"Authenticator": {
3-
"version": "2.0.1",
4-
"build": "19010601"
3+
"version": "2.1.0",
4+
"build": "19012201"
55
}
66
}

0 commit comments

Comments
 (0)