Skip to content

Commit 79d018c

Browse files
authored
Merge pull request #550 from OpenConext/bugfix/add-prove-possession-for-already-registered-tokens
Check already registered on prove possession
2 parents 946dddc + 0f7f8fd commit 79d018c

File tree

4 files changed

+509
-5
lines changed

4 files changed

+509
-5
lines changed

ci/qa/phpstan-baseline.neon

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1392,7 +1392,7 @@ parameters:
13921392

13931393
-
13941394
message: "#^Argument of an invalid type Surfnet\\\\Stepup\\\\Identity\\\\Entity\\\\SecondFactorCollection\\|null supplied for foreach, only iterables are supported\\.$#"
1395-
count: 8
1395+
count: 11
13961396
path: ../../src/Surfnet/Stepup/Identity/Identity.php
13971397

13981398
-
@@ -1455,6 +1455,11 @@ parameters:
14551455
count: 1
14561456
path: ../../src/Surfnet/Stepup/Identity/Identity.php
14571457

1458+
-
1459+
message: "#^Cannot call method getType\\(\\) on mixed\\.$#"
1460+
count: 3
1461+
path: ../../src/Surfnet/Stepup/Identity/Identity.php
1462+
14581463
-
14591464
message: "#^Cannot call method getValues\\(\\) on Surfnet\\\\Stepup\\\\Identity\\\\Entity\\\\RegistrationAuthorityCollection\\|null\\.$#"
14601465
count: 1

src/Surfnet/Stepup/Identity/Identity.php

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ public function bootstrapYubikeySecondFactor(
210210
): void {
211211
$this->assertNotForgotten();
212212
$this->assertUserMayAddSecondFactor($maxNumberOfTokens);
213+
$this->assertTokenTypeNotAlreadyRegistered(new SecondFactorType('yubikey'));
213214

214215
$this->apply(
215216
new YubikeySecondFactorBootstrappedEvent(
@@ -234,6 +235,7 @@ public function provePossessionOfYubikey(
234235
): void {
235236
$this->assertNotForgotten();
236237
$this->assertUserMayAddSecondFactor($maxNumberOfTokens);
238+
$this->assertTokenTypeNotAlreadyRegistered(new SecondFactorType('yubikey'));
237239

238240
if ($emailVerificationRequired) {
239241
$emailVerificationNonce = TokenGenerator::generateNonce();
@@ -278,6 +280,7 @@ public function provePossessionOfPhone(
278280
): void {
279281
$this->assertNotForgotten();
280282
$this->assertUserMayAddSecondFactor($maxNumberOfTokens);
283+
$this->assertTokenTypeNotAlreadyRegistered(new SecondFactorType('sms'));
281284

282285
if ($emailVerificationRequired) {
283286
$emailVerificationNonce = TokenGenerator::generateNonce();
@@ -373,6 +376,7 @@ public function provePossessionOfGssf(
373376
): void {
374377
$this->assertNotForgotten();
375378
$this->assertUserMayAddSecondFactor($maxNumberOfTokens);
379+
$this->assertTokenTypeNotAlreadyRegistered(new SecondFactorType($provider->getStepupProvider()));
376380

377381
if ($emailVerificationRequired) {
378382
$emailVerificationNonce = TokenGenerator::generateNonce();
@@ -422,6 +426,7 @@ public function provePossessionOfU2fDevice(
422426
): void {
423427
$this->assertNotForgotten();
424428
$this->assertUserMayAddSecondFactor($maxNumberOfTokens);
429+
$this->assertTokenTypeNotAlreadyRegistered(new SecondFactorType('u2f'));
425430

426431
if ($emailVerificationRequired) {
427432
$emailVerificationNonce = TokenGenerator::generateNonce();
@@ -685,6 +690,7 @@ public function migrateVettedSecondFactor(
685690
throw new DomainException("The second factor on the original identity can not be found");
686691
}
687692
$this->assertTokenNotAlreadyRegistered($secondFactor->getType(), $secondFactor->getIdentifier());
693+
$this->assertTokenTypeNotAlreadyRegistered($secondFactor->getType());
688694
if ($sourceIdentity->getInstitution()->equals($this->getInstitution())) {
689695
throw new DomainException("Cannot move the second factor to the same institution");
690696
}
@@ -1546,21 +1552,40 @@ private function assertTokenNotAlreadyRegistered(SecondFactorType $type, SecondF
15461552
{
15471553
foreach ($this->unverifiedSecondFactors as $unverified) {
15481554
if ($unverified->typeAndIdentifierAreEqual($type, $identifier)) {
1549-
throw new DomainException("The second factor was already registered as a unverified second factor");
1555+
throw new DomainException("The second factor was already registered as an unverified second factor");
15501556
}
15511557
}
15521558
foreach ($this->verifiedSecondFactors as $verified) {
15531559
if ($verified->typeAndIdentifierAreEqual($type, $identifier)) {
15541560
throw new DomainException("The second factor was already registered as a verified second factor");
15551561
}
15561562
}
1557-
foreach ($this->vettedSecondFactors as $vettedSecondFactor) {
1558-
if ($vettedSecondFactor->typeAndIdentifierAreEqual($type, $identifier)) {
1563+
foreach ($this->vettedSecondFactors as $vetted) {
1564+
if ($vetted->typeAndIdentifierAreEqual($type, $identifier)) {
15591565
throw new DomainException("The second factor was registered as a vetted second factor");
15601566
}
15611567
}
15621568
}
15631569

1570+
private function assertTokenTypeNotAlreadyRegistered(SecondFactorType $type): void
1571+
{
1572+
foreach ($this->unverifiedSecondFactors as $unverified) {
1573+
if ($unverified->getType()->equals($type)) {
1574+
throw new DomainException("This second factor type was already registered as an unverified second factor");
1575+
}
1576+
}
1577+
foreach ($this->verifiedSecondFactors as $verified) {
1578+
if ($verified->getType()->equals($type)) {
1579+
throw new DomainException("This second factor type was already registered as a verified second factor");
1580+
}
1581+
}
1582+
foreach ($this->vettedSecondFactors as $vetted) {
1583+
if ($vetted->getType()->equals($type)) {
1584+
throw new DomainException("This second factor type was already registered as a vetted second factor");
1585+
}
1586+
}
1587+
}
1588+
15641589
private function assertSelfAssertedTokenRegistrationAllowed(): void
15651590
{
15661591
if ($this->vettedSecondFactors->count() !== 0) {

src/Surfnet/StepupMiddleware/CommandHandlingBundle/Tests/Identity/CommandHandler/IdentityCommandHandlerMoveTokenTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,7 @@ public function test_can_not_be_moved_if_already_present_as_verified_token(): vo
593593

594594
public function test_can_not_be_moved_if_already_present_as_unverified_token(): void
595595
{
596-
$this->expectExceptionMessage("The second factor was already registered as a unverified second factor");
596+
$this->expectExceptionMessage("The second factor was already registered as an unverified second factor");
597597
$this->expectException(DomainException::class);
598598

599599
$this->setUpInstitutionConfiguration(2, ['yubikey']);

0 commit comments

Comments
 (0)