Skip to content

Commit 5503dd8

Browse files
nuxsminnuxsmin
committed
* [FIX] Wrong behavior when upgrading from v3.0. Thanks to @Envikia for the feedback. Related #1401
* [MOD] Improved error code when an unknown API token is used. Thanks to @matejzero for the feedback. Closes #1429 * [FIX] Wrong behavior when changing master password and there aren't any accounts for processing. Thanks to @matejzero for the feedback. Closes #1430 Signed-off-by: Rubén D <[email protected]>
1 parent 5eaaac4 commit 5503dd8

File tree

4 files changed

+72
-34
lines changed

4 files changed

+72
-34
lines changed

‎lib/SP/Services/Account/AccountCryptService.php

+47-22
Original file line numberDiff line numberDiff line change
@@ -70,23 +70,34 @@ public function updateMasterPassword(UpdateMasterPassRequest $updateMasterPassRe
7070
$this->request = $updateMasterPassRequest;
7171

7272
try {
73-
$this->eventDispatcher->notifyEvent('update.masterPassword.accounts.start',
74-
new Event($this, EventMessage::factory()->addDescription(__u('Update Master Password')))
73+
$this->eventDispatcher->notifyEvent(
74+
'update.masterPassword.accounts.start',
75+
new Event($this,
76+
EventMessage::factory()
77+
->addDescription(__u('Update Master Password'))
78+
)
7579
);
7680

7781
if ($this->request->useTask()) {
7882
$task = $this->request->getTask();
7983

8084
TaskFactory::update($task,
81-
TaskFactory::createMessage($task->getTaskId(), __u('Update Master Password'))
85+
TaskFactory::createMessage($task->getTaskId(),
86+
__u('Update Master Password'))
8287
);
8388
}
8489

85-
$eventMessage = $this->processAccounts($this->accountService->getAccountsPassData(), function ($request) {
86-
$this->accountService->updatePasswordMasterPass($request);
87-
});
90+
$eventMessage = $this->processAccounts(
91+
$this->accountService->getAccountsPassData(),
92+
function (AccountPasswordRequest $request) {
93+
$this->accountService->updatePasswordMasterPass($request);
94+
}
95+
);
8896

89-
$this->eventDispatcher->notifyEvent('update.masterPassword.accounts.end', new Event($this, $eventMessage));
97+
$this->eventDispatcher->notifyEvent(
98+
'update.masterPassword.accounts.end',
99+
new Event($this, $eventMessage)
100+
);
90101
} catch (Exception $e) {
91102
$this->eventDispatcher->notifyEvent('exception', new Event($e));
92103

@@ -104,7 +115,6 @@ public function updateMasterPassword(UpdateMasterPassRequest $updateMasterPassRe
104115
* @param callable $passUpdater
105116
*
106117
* @return EventMessage
107-
* @throws ServiceException
108118
*/
109119
private function processAccounts(array $accounts, callable $passUpdater)
110120
{
@@ -116,8 +126,14 @@ private function processAccounts(array $accounts, callable $passUpdater)
116126
$startTime = time();
117127
$numAccounts = count($accounts);
118128

129+
$eventMessage = EventMessage::factory();
130+
119131
if ($numAccounts === 0) {
120-
throw new ServiceException(__u('Error while retrieving the accounts\' passwords'), ServiceException::ERROR);
132+
$eventMessage->addDescription(__u('There are no accounts for processing'));
133+
$eventMessage->addDetail(__u('Accounts updated'), __u('N/A'));
134+
$eventMessage->addDetail(__u('Errors'), 0);
135+
136+
return $eventMessage;
121137
}
122138

123139
$configData = $this->config->getConfigData();
@@ -127,8 +143,6 @@ private function processAccounts(array $accounts, callable $passUpdater)
127143
$task = $this->request->getTask();
128144
}
129145

130-
$eventMessage = EventMessage::factory();
131-
132146
foreach ($accounts as $account) {
133147
// No realizar cambios si está en modo demo
134148
if ($configData->isDemoEnabled()) {
@@ -140,8 +154,10 @@ private function processAccounts(array $accounts, callable $passUpdater)
140154
$eta = Util::getETA($startTime, $counter, $numAccounts);
141155

142156
if (isset($task)) {
143-
$taskMessage = TaskFactory::createMessage($task->getTaskId(), __('Update Master Password'))
144-
->setMessage(sprintf(__('Accounts updated: %d / %d'), $counter, $numAccounts))
157+
$taskMessage = TaskFactory::createMessage(
158+
$task->getTaskId(),
159+
__('Update Master Password')
160+
)->setMessage(sprintf(__('Accounts updated: %d / %d'), $counter, $numAccounts))
145161
->setProgress(round(($counter * 100) / $numAccounts, 2))
146162
->setTime(sprintf('ETA: %ds (%.2f/s)', $eta[0], $eta[1]));
147163

@@ -213,26 +229,35 @@ public function updateHistoryMasterPassword(UpdateMasterPassRequest $updateMaste
213229
$this->request = $updateMasterPassRequest;
214230

215231
try {
216-
$this->eventDispatcher->notifyEvent('update.masterPassword.accountsHistory.start',
217-
new Event($this, EventMessage::factory()->addDescription(__u('Update Master Password (H)')))
232+
$this->eventDispatcher->notifyEvent(
233+
'update.masterPassword.accountsHistory.start',
234+
new Event($this,
235+
EventMessage::factory()
236+
->addDescription(__u('Update Master Password (H)'))
237+
)
218238
);
219239

220240
if ($this->request->useTask()) {
221241
$task = $this->request->getTask();
222242

223243
TaskFactory::update($task,
224-
TaskFactory::createMessage($task->getTaskId(), __u('Update Master Password (H)'))
244+
TaskFactory::createMessage($task->getTaskId(),
245+
__u('Update Master Password (H)'))
225246
);
226247
}
227248

228-
$eventMessage = $this->processAccounts($this->accountHistoryService->getAccountsPassData(), function ($request) {
229-
/** @var AccountPasswordRequest $request */
230-
$request->hash = $this->request->getHash();
249+
$eventMessage = $this->processAccounts(
250+
$this->accountHistoryService->getAccountsPassData(),
251+
function (AccountPasswordRequest $request) {
252+
$request->hash = $this->request->getHash();
231253

232-
$this->accountHistoryService->updatePasswordMasterPass($request);
233-
});
254+
$this->accountHistoryService->updatePasswordMasterPass($request);
255+
});
234256

235-
$this->eventDispatcher->notifyEvent('update.masterPassword.accountsHistory.end', new Event($this, $eventMessage));
257+
$this->eventDispatcher->notifyEvent(
258+
'update.masterPassword.accountsHistory.end',
259+
new Event($this, $eventMessage)
260+
);
236261
} catch (Exception $e) {
237262
$this->eventDispatcher->notifyEvent('exception', new Event($e));
238263

‎lib/SP/Services/Api/ApiService.php

+14-1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
use SP\Core\Exceptions\SPException;
3434
use SP\DataModel\AuthTokenData;
3535
use SP\Modules\Api\Controllers\Help\HelpInterface;
36+
use SP\Repositories\NoSuchItemException;
3637
use SP\Repositories\Track\TrackRequest;
3738
use SP\Services\AuthToken\AuthTokenService;
3839
use SP\Services\Service;
@@ -103,7 +104,19 @@ public function setup($actionId)
103104
);
104105
}
105106

106-
$this->authTokenData = $this->authTokenService->getTokenByToken($actionId, $this->getParam('authToken'));
107+
try {
108+
$this->authTokenData = $this->authTokenService->getTokenByToken($actionId, $this->getParam('authToken'));
109+
} catch (NoSuchItemException $e) {
110+
logger($e->getMessage(), 'ERROR');
111+
112+
// For security reasons there won't be any hint about a not found token...
113+
throw new ServiceException(
114+
__u('Internal error'),
115+
ServiceException::ERROR,
116+
null,
117+
JsonRpcResponse::INTERNAL_ERROR
118+
);
119+
}
107120

108121
if ($this->authTokenData->getActionId() !== $actionId) {
109122
$this->accessDenied();

‎lib/SP/Services/AuthToken/AuthTokenService.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -307,15 +307,15 @@ public function updateRaw(AuthTokenData $itemData)
307307
*
308308
* @return false|AuthTokenData
309309
* @throws ConstraintException
310+
* @throws NoSuchItemException
310311
* @throws QueryException
311-
* @throws ServiceException
312312
*/
313313
public function getTokenByToken($actionId, $token)
314314
{
315315
$result = $this->authTokenRepository->getTokenByToken($actionId, $token);
316316

317317
if ($result->getNumRows() === 0) {
318-
throw new ServiceException(__u('Internal error'));
318+
throw new NoSuchItemException(__u('Token not found'));
319319
}
320320

321321
return $result->getData();

‎schemas/31019012201.sql

+9-9
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@ alter table Plugin
55

66
create table PluginData
77
(
8-
name varchar(100) not null,
9-
itemId int not null,
10-
`data` blob not null,
11-
`key` varbinary(2000) not null,
12-
constraint `PRIMARY`
8+
name varchar(100) not null,
9+
itemId int not null,
10+
`data` blob not null,
11+
`key` varbinary(2000) not null,
1312
primary key (name, itemId),
14-
constraint fk_PluginData_name
15-
foreign key (name) references Plugin (name)
16-
on update cascade on delete cascade
17-
)$$
13+
constraint fk_PluginData_name
14+
foreign key (name) references Plugin (name)
15+
on update cascade
16+
on delete cascade
17+
) ENGINE = InnoDB DEFAULT CHARSET = utf8 COLLATE utf8_unicode_ci $$

0 commit comments

Comments
 (0)