Skip to content

Commit 67979b8

Browse files
authored
Merge pull request #2585 from LibreSign/feature/bugfix-retrieve-sign-method-of-multiple-signers
Retrieve signer data of multiple signers
2 parents a719e26 + d2c197e commit 67979b8

File tree

10 files changed

+119
-45
lines changed

10 files changed

+119
-45
lines changed

lib/Service/IdentifyMethod/AbstractIdentifyMethod.php

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ abstract class AbstractIdentifyMethod implements IIdentifyMethod {
4343
protected string $codeSentByUser = '';
4444
protected array $settings = [];
4545
protected bool $willNotify = true;
46+
/**
47+
* @var string[]
48+
*/
49+
public array $availableSignatureMethods;
4650
/**
4751
* @var AbstractSignatureMethod[]
4852
*/
@@ -89,11 +93,28 @@ public function signatureMethodsToArray(): array {
8993
return array_map(function (AbstractSignatureMethod $method) {
9094
return [
9195
'label' => $method->friendlyName,
96+
'name' => $method->getName(),
9297
'enabled' => $method->isEnabled(),
9398
];
9499
}, $this->signatureMethods);
95100
}
96101

102+
public function getEmptyInstanceOfSignatureMethodByName(string $name): AbstractSignatureMethod {
103+
if (!in_array($name, $this->availableSignatureMethods)) {
104+
throw new InvalidArgumentException(sprintf('%s is not a valid signature method of identify method %s', $name, $this->getName()));
105+
}
106+
$className = 'OCA\Libresign\Service\IdentifyMethod\\SignatureMethod\\' . ucfirst($name);
107+
if (!class_exists($className)) {
108+
throw new InvalidArgumentException('Invalid signature method. Set at identify method the list of available signature methdos with right values.');
109+
}
110+
$signatureMethod = clone \OC::$server->get($className);
111+
$signatureMethod->cleanEntity();
112+
return $signatureMethod;
113+
}
114+
115+
/**
116+
* @return AbstractSignatureMethod[]
117+
*/
97118
public function getSignatureMethods(): array {
98119
return $this->signatureMethods;
99120
}
@@ -294,14 +315,23 @@ private function loadSavedSettings(): void {
294315
}
295316
return $carry;
296317
}, []);
318+
foreach ($this->availableSignatureMethods as $signatureMethodName) {
319+
$this->signatureMethods[$signatureMethodName] =
320+
$this->getEmptyInstanceOfSignatureMethodByName($signatureMethodName);
321+
}
297322
if (!isset($this->settings['signatureMethods']) || !is_array($this->settings['signatureMethods'])) {
298323
return;
299324
}
300-
foreach ($this->settings['signatureMethods'] as $method => $settings) {
301-
$this->signatureMethods[$method]->setEntity($this->getEntity());
302-
if (is_object($this->signatureMethods[$method]) && isset($settings['enabled']) && $settings['enabled']) {
303-
$this->signatureMethods[$method]->enable();
325+
foreach ($this->settings['signatureMethods'] as $signatureMethodName => $settings) {
326+
if (!in_array($signatureMethodName, $this->availableSignatureMethods)) {
327+
unset($this->settings['signatureMethods'][$signatureMethodName]);
328+
continue;
329+
}
330+
$signatureMethod = $this->getEmptyInstanceOfSignatureMethodByName($signatureMethodName);
331+
if (isset($settings['enabled']) && $settings['enabled']) {
332+
$signatureMethod->enable();
304333
}
334+
$this->signatureMethods[$signatureMethodName] = $signatureMethod;
305335
}
306336
}
307337

lib/Service/IdentifyMethod/Account.php

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,7 @@
2727
use OCA\Libresign\Db\IdentifyMethodMapper;
2828
use OCA\Libresign\Exception\LibresignException;
2929
use OCA\Libresign\Helper\JSActions;
30-
use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\ClickToSign;
31-
use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\EmailToken;
32-
use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\Password;
30+
use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\ISignatureMethod;
3331
use OCA\Libresign\Service\MailService;
3432
use OCA\Libresign\Service\SessionService;
3533
use OCP\AppFramework\Utility\ITimeFactory;
@@ -43,6 +41,11 @@
4341
use Psr\Log\LoggerInterface;
4442

4543
class Account extends AbstractIdentifyMethod {
44+
public array $availableSignatureMethods = [
45+
ISignatureMethod::SIGNATURE_METHOD_CLICK_TO_SIGN,
46+
ISignatureMethod::SIGNATURE_METHOD_EMAIL_TOKEN,
47+
ISignatureMethod::SIGNATURE_METHOD_PASSWORD,
48+
];
4649
public function __construct(
4750
protected IdentifyService $identifyService,
4851
private IUserManager $userManager,
@@ -56,17 +59,9 @@ public function __construct(
5659
private LoggerInterface $logger,
5760
private SessionService $sessionService,
5861
private MailService $mail,
59-
private Password $password,
60-
private ClickToSign $clickToSign,
61-
private EmailToken $emailToken,
6262
) {
6363
// TRANSLATORS Name of possible authenticator method. This signalize that the signer could be identified by Nextcloud acccount
6464
$this->friendlyName = $this->identifyService->getL10n()->t('Account');
65-
$this->signatureMethods = [
66-
$this->password->getName() => $this->password,
67-
$this->clickToSign->getName() => $this->clickToSign,
68-
$this->emailToken->getName() => $this->emailToken,
69-
];
7065
parent::__construct(
7166
$identifyService,
7267
);

lib/Service/IdentifyMethod/Email.php

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,32 +28,29 @@
2828
use OCA\Libresign\Db\IdentifyMethodMapper;
2929
use OCA\Libresign\Exception\LibresignException;
3030
use OCA\Libresign\Helper\JSActions;
31-
use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\ClickToSign;
32-
use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\EmailToken;
31+
use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\ISignatureMethod;
3332
use OCA\Libresign\Service\SessionService;
3433
use OCP\AppFramework\Utility\ITimeFactory;
3534
use OCP\Files\IRootFolder;
3635
use OCP\IUser;
3736
use OCP\IUserSession;
3837

3938
class Email extends AbstractIdentifyMethod {
39+
public array $availableSignatureMethods = [
40+
ISignatureMethod::SIGNATURE_METHOD_CLICK_TO_SIGN,
41+
ISignatureMethod::SIGNATURE_METHOD_EMAIL_TOKEN,
42+
];
4043
public function __construct(
4144
protected IdentifyService $identifyService,
4245
private IdentifyMethodMapper $identifyMethodMapper,
4346
private IRootFolder $root,
4447
private ITimeFactory $timeFactory,
4548
private SessionService $sessionService,
4649
private FileElementMapper $fileElementMapper,
47-
private ClickToSign $clickToSign,
48-
private EmailToken $emailToken,
4950
private IUserSession $userSession,
5051
) {
5152
// TRANSLATORS Name of possible authenticator method. This signalize that the signer could be identified by email
5253
$this->friendlyName = $this->identifyService->getL10n()->t('Email');
53-
$this->signatureMethods = [
54-
$this->clickToSign->getName() => $this->clickToSign,
55-
$this->emailToken->getName() => $this->emailToken,
56-
];
5754
parent::__construct(
5855
$identifyService,
5956
);

lib/Service/IdentifyMethod/IIdentifyMethod.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,7 @@ public function setCodeSentByUser(string $code): void;
3636
public function cleanEntity(): void;
3737
public function setEntity(IdentifyMethod $entity): void;
3838
public function getEntity(): IdentifyMethod;
39-
/**
40-
* @return AbstractSignatureMethod[]
41-
*/
39+
public function getEmptyInstanceOfSignatureMethodByName(string $name): AbstractSignatureMethod;
4240
public function getSignatureMethods(): array;
4341
public function signatureMethodsToArray(): array;
4442
public function getSettings(): array;

lib/Service/IdentifyMethod/SignatureMethod/ISignatureMethod.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@
2525
namespace OCA\Libresign\Service\IdentifyMethod\SignatureMethod;
2626

2727
interface ISignatureMethod {
28+
public const SIGNATURE_METHOD_CLICK_TO_SIGN = 'clickToSign';
29+
public const SIGNATURE_METHOD_EMAIL_TOKEN = 'emailToken';
30+
public const SIGNATURE_METHOD_PASSWORD = 'password';
2831
public function enable(): void;
2932
public function isEnabled(): bool;
3033
public function toArray(): array;

lib/Service/IdentifyMethodService.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,12 @@ public function getSignMethodsOfIdentifiedFactors(int $signRequestId): array {
196196
if (empty($identifyMethod->getEntity()->getIdentifiedAtDate())) {
197197
continue;
198198
}
199-
foreach ($identifyMethod->getSignatureMethods() as $signatureMethod) {
199+
$signatureMethods = $identifyMethod->getSignatureMethods();
200+
foreach ($signatureMethods as $signatureMethod) {
200201
if (!$signatureMethod->isEnabled()) {
201202
continue;
202203
}
204+
$signatureMethod->setEntity($identifyMethod->getEntity());
203205
$return[$signatureMethod->getName()] = $signatureMethod->toArray();
204206
}
205207
}

lib/Service/SignFileService.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -447,12 +447,13 @@ public function requestCode(
447447
throw new LibresignException($this->l10n->t('Invalid identification method'));
448448
}
449449
foreach ($identifyMethods[$identifyMethodName] as $identifyMethod) {
450-
$signatureMethods = $identifyMethod->getSignatureMethods();
451-
if (empty($signatureMethods[$signMethodName])) {
452-
throw new LibresignException($this->l10n->t('Invalid identification method'));
450+
try {
451+
$signatureMethod = $identifyMethod->getEmptyInstanceOfSignatureMethodByName($signMethodName);
452+
$signatureMethod->setEntity($identifyMethod->getEntity());
453+
} catch (InvalidArgumentException $th) {
454+
continue;
453455
}
454456
/** @var EmailToken $signatureMethod */
455-
$signatureMethod = $signatureMethods[$signMethodName];
456457
$signatureMethod->requestCode($identify);
457458
return;
458459
}

tests/integration/features/admin/initial_state.feature

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ Feature: admin/initial_state
66
Then the response should contain the initial state "libresign-identify_methods" with the following values:
77
"""
88
[
9-
{"name":"account","friendly_name":"Account","enabled":true,"mandatory":true,"signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign"},"emailToken":{"enabled":false,"label":"Email token"},"password":{"enabled":false,"label":"Certificate with password"}}},
10-
{"name":"email","friendly_name":"Email","enabled":false,"mandatory":true,"can_create_account":true,"test_url":"/index.php/settings/admin/mailtest","signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign"},"emailToken":{"enabled":false,"label":"Email token"}}}
9+
{"name":"account","friendly_name":"Account","enabled":true,"mandatory":true,"signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign","name":"clickToSign"},"emailToken":{"enabled":false,"label":"Email token","name":"emailToken"},"password":{"enabled":false,"label":"Certificate with password","name":"password"}}},
10+
{"name":"email","friendly_name":"Email","enabled":false,"mandatory":true,"can_create_account":true,"test_url":"/index.php/settings/admin/mailtest","signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign","name":"clickToSign"},"emailToken":{"enabled":false,"label":"Email token","name":"emailToken"}}}
1111
]
1212
"""
1313

@@ -20,8 +20,8 @@ Feature: admin/initial_state
2020
And the response should contain the initial state "libresign-identify_methods" with the following values:
2121
"""
2222
[
23-
{"name":"account","friendly_name":"Account","enabled":true,"mandatory":true,"signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign"},"emailToken":{"enabled":false,"label":"Email token"},"password":{"enabled":false,"label":"Certificate with password"}}},
24-
{"name":"email","friendly_name":"Email","enabled":false,"mandatory":true,"can_create_account":true,"test_url":"/index.php/settings/admin/mailtest","signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign"},"emailToken":{"enabled":false,"label":"Email token"}}}
23+
{"name":"account","friendly_name":"Account","enabled":true,"mandatory":true,"signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign","name":"clickToSign"},"emailToken":{"enabled":false,"label":"Email token","name":"emailToken"},"password":{"enabled":false,"label":"Certificate with password","name":"password"}}},
24+
{"name":"email","friendly_name":"Email","enabled":false,"mandatory":true,"can_create_account":true,"test_url":"/index.php/settings/admin/mailtest","signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign","name":"clickToSign"},"emailToken":{"enabled":false,"label":"Email token","name":"emailToken"}}}
2525
]
2626
"""
2727

@@ -34,8 +34,8 @@ Feature: admin/initial_state
3434
And the response should contain the initial state "libresign-identify_methods" with the following values:
3535
"""
3636
[
37-
{"name":"account","friendly_name":"Account","enabled":true,"mandatory":true,"signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign"},"emailToken":{"enabled":false,"label":"Email token"},"password":{"enabled":false,"label":"Certificate with password"}}},
38-
{"name":"email","friendly_name":"Email","enabled":false,"mandatory":true,"can_create_account":true,"test_url":"/index.php/settings/admin/mailtest","signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign"},"emailToken":{"enabled":false,"label":"Email token"}}}
37+
{"name":"account","friendly_name":"Account","enabled":true,"mandatory":true,"signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign","name":"clickToSign"},"emailToken":{"enabled":false,"label":"Email token","name":"emailToken"},"password":{"enabled":false,"label":"Certificate with password","name":"password"}}},
38+
{"name":"email","friendly_name":"Email","enabled":false,"mandatory":true,"can_create_account":true,"test_url":"/index.php/settings/admin/mailtest","signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign","name":"clickToSign"},"emailToken":{"enabled":false,"label":"Email token","name":"emailToken"}}}
3939
]
4040
"""
4141

@@ -48,8 +48,8 @@ Feature: admin/initial_state
4848
And the response should contain the initial state "libresign-identify_methods" with the following values:
4949
"""
5050
[
51-
{"name":"account","friendly_name":"Account","enabled":true,"mandatory":true,"signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign"},"emailToken":{"enabled":false,"label":"Email token"},"password":{"enabled":false,"label":"Certificate with password"}}},
52-
{"name":"email","friendly_name":"Email","enabled":false,"mandatory":true,"can_create_account":true,"test_url":"/index.php/settings/admin/mailtest","signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign"},"emailToken":{"enabled":false,"label":"Email token"}}}
51+
{"name":"account","friendly_name":"Account","enabled":true,"mandatory":true,"signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign","name":"clickToSign"},"emailToken":{"enabled":false,"label":"Email token","name":"emailToken"},"password":{"enabled":false,"label":"Certificate with password","name":"password"}}},
52+
{"name":"email","friendly_name":"Email","enabled":false,"mandatory":true,"can_create_account":true,"test_url":"/index.php/settings/admin/mailtest","signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign","name":"clickToSign"},"emailToken":{"enabled":false,"label":"Email token","name":"emailToken"}}}
5353
]
5454
"""
5555

@@ -62,8 +62,8 @@ Feature: admin/initial_state
6262
And the response should contain the initial state "libresign-identify_methods" with the following values:
6363
"""
6464
[
65-
{"name":"account","friendly_name":"Account","enabled":true,"mandatory":true,"signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign"},"emailToken":{"enabled":false,"label":"Email token"},"password":{"enabled":false,"label":"Certificate with password"}}},
66-
{"name":"email","friendly_name":"Email","enabled":false,"mandatory":true,"can_create_account":true,"test_url":"/index.php/settings/admin/mailtest","signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign"},"emailToken":{"enabled":false,"label":"Email token"}}}
65+
{"name":"account","friendly_name":"Account","enabled":true,"mandatory":true,"signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign","name":"clickToSign"},"emailToken":{"enabled":false,"label":"Email token","name":"emailToken"},"password":{"enabled":false,"label":"Certificate with password","name":"password"}}},
66+
{"name":"email","friendly_name":"Email","enabled":false,"mandatory":true,"can_create_account":true,"test_url":"/index.php/settings/admin/mailtest","signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign","name":"clickToSign"},"emailToken":{"enabled":false,"label":"Email token","name":"emailToken"}}}
6767
]
6868
"""
6969

@@ -75,7 +75,7 @@ Feature: admin/initial_state
7575
And the response should contain the initial state "libresign-identify_methods" with the following values:
7676
"""
7777
[
78-
{"name":"account","friendly_name":"Account","enabled":true,"mandatory":true,"signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign"},"emailToken":{"enabled":false,"label":"Email token"},"password":{"enabled":false,"label":"Certificate with password"}}},
79-
{"name":"email","friendly_name":"Email","enabled":false,"mandatory":false,"can_create_account":true,"test_url":"/index.php/settings/admin/mailtest","signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign"},"emailToken":{"enabled":false,"label":"Email token"}}}
78+
{"name":"account","friendly_name":"Account","enabled":true,"mandatory":true,"signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign","name":"clickToSign"},"emailToken":{"enabled":false,"label":"Email token","name":"emailToken"},"password":{"enabled":false,"label":"Certificate with password","name":"password"}}},
79+
{"name":"email","friendly_name":"Email","enabled":false,"mandatory":false,"can_create_account":true,"test_url":"/index.php/settings/admin/mailtest","signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign","name":"clickToSign"},"emailToken":{"enabled":false,"label":"Email token","name":"emailToken"}}}
8080
]
8181
"""

tests/integration/features/page/index.feature

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ Feature: page/sign_identify_default
1010
And the response should contain the initial state "libresign-identify_methods" with the following values:
1111
"""
1212
[
13-
{"name":"account","friendly_name":"Account","enabled":true,"mandatory":true,"signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign"},"emailToken":{"enabled":false,"label":"Email token"},"password":{"enabled":false,"label":"Certificate with password"}}},
14-
{"name":"email","friendly_name":"Email","enabled":false,"mandatory":true,"can_create_account":true,"test_url":"/index.php/settings/admin/mailtest","signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign"},"emailToken":{"enabled":false,"label":"Email token"}}}
13+
{"name":"account","friendly_name":"Account","enabled":true,"mandatory":true,"signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign","name":"clickToSign"},"emailToken":{"enabled":false,"label":"Email token","name":"emailToken"},"password":{"enabled":false,"label":"Certificate with password","name":"password"}}},
14+
{"name":"email","friendly_name":"Email","enabled":false,"mandatory":true,"can_create_account":true,"test_url":"/index.php/settings/admin/mailtest","signatureMethods":{"clickToSign":{"enabled":false,"label":"Click to sign","name":"clickToSign"},"emailToken":{"enabled":false,"label":"Email token","name":"emailToken"}}}
1515
]
1616
"""

0 commit comments

Comments
 (0)