Adjust and bugfix self service features

Points brought up by running the global behat tests

config/packages/monolog.yaml

@@ -1,54 +1,29 @@
 monolog:
-  channels:
-    - deprecation # Deprecations are logged in the dedicated "deprecation" channel when it exists
   handlers:
     prod-signaler:
       type: fingers_crossed
       action_level: ERROR
       passthru_level: NOTICE # this means that all message of level NOTICE or higher are always logged
-      handler: main_stdout
       channels: [ "!event" ]
+      handler: main_syslog
       bubble: false # if we handle it, nothing else should
-    main_stdout:
+    main_syslog:
       type: stream
-      ident: stepup-selfservice
       path: "php://stderr"
       formatter: surfnet_stepup.monolog.json_formatter
+      channels: [ "!event" ]
 
 when@dev:
-  monolog:
-    handlers:
-      prod-signaler:
-        action_level: ERROR
-        passthru_level: DEBUG # DEV setting: this means that all message of level DEBUG or higher are always logged
-        channels: [ "!event" ]
-        bubble: true
-      main_logfile:
-        type: stream
-        handler: logfile
-        level: NOTICE
-        path: "%kernel.logs_dir%/%kernel.environment%.log"
-      deprecation:
-        type: rotating_file
-        path: "%kernel.logs_dir%/%kernel.environment%.deprecations.log"
-        max_files: 2
-        channels: [ deprecation ]
-
 when@test:
   monolog:
     handlers:
-      main:
-        type: fingers_crossed
-        action_level: error
-        handler: nested
-        excluded_http_codes: [ 404, 405 ]
-        channels: [ "!event" ]
-      deprecation:
-        type: rotating_file
-        path: "%kernel.logs_dir%/%kernel.environment%.deprecations.log"
-        max_files: 2
-        channels: [ deprecation ]
-      nested:
+      main_syslog:
         type: stream
-        path: "%kernel.logs_dir%/%kernel.environment%.log"
-        level: debug
+        path: php://stderr
+        level: error
+        channels: ["!event", "!doctrine", "!deprecation", "!console"]
+      console:
+        type: console
+        process_psr_3_messages: false
+        channels: ["!event", "!doctrine", "!deprecation", "!console"]
+

config/services.yaml

@@ -40,6 +40,11 @@ services:
       - []
       - '@logger'
 
+  Surfnet\StepupSelfService\SelfServiceBundle\Service\SmsSecondFactorServiceInterface:
+    class: Surfnet\StepupSelfService\SelfServiceBundle\Service\SmsSecondFactorService
+    arguments:
+      $commandService: '@surfnet_stepup_self_service_self_service.service.command'
+
   Surfnet\StepupSelfService\SamlStepupProviderBundle\Session\SessionFactoryWithAttributeBag:
     decorates: session.factory
     arguments: [ '@.inner' ]

config/services_smoketest.yaml

@@ -5,17 +5,27 @@ parameters:
   middleware_credentials_password: secret
 
 services:
-  surfnet_stepup_self_service_self_service.service.sms_second_factor:
+
+  _defaults:
+    autowire: true      # Automatically injects dependencies in your services.
+    autoconfigure: true # Automatically registers your services as commands, event subscribers, etc.
+    public: false
+
+  Surfnet\StepupSelfService\SelfServiceBundle\Service\SmsSecondFactorServiceInterface:
     class: Surfnet\StepupSelfService\SelfServiceBundle\Tests\TestDouble\Service\SmsSecondFactorService
     arguments:
         - "@surfnet_stepup_self_service_self_service.service.command"
 
+  Surfnet\StepupSelfService\SelfServiceBundle\Service\YubikeySecondFactorServiceInterface:
+    class: Surfnet\StepupSelfService\SelfServiceBundle\Tests\TestDouble\Service\YubikeySecondFactorService
+    arguments:
+      $commandService: "@surfnet_stepup_self_service_self_service.service.command"
+      $logger: "@logger"
 
   # The middleware client bundle guzzle client is overloaded to be able to pass the testcookie to the ensure MW is
   # loaded in test mode. This way people setting the testcookie in prod will not switch their mw api into testmode
   # resulting in 500 errors.
   surfnet_stepup_middleware_client.guzzle.api:
-    public: false
     class: GuzzleHttp\Client
     factory: ['Surfnet\StepupSelfService\SelfServiceBundle\Tests\TestDouble\Factory\GuzzleApiFactory', createApiGuzzleClient]
     arguments:
@@ -24,8 +34,7 @@ services:
         - "%middleware_credentials_password%"
 
   surfnet_stepup_middleware_client.guzzle.commands:
-    public: false
     class: GuzzleHttp\Client
     factory: ['Surfnet\StepupSelfService\SelfServiceBundle\Tests\TestDouble\Factory\GuzzleApiFactory', createCommandGuzzleClient]
     arguments:
-        - "%middleware_url_command_api%"
+        - "%middleware_url_command_api%"

src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/Registration/Sms/SmsProofPossessionController.php

@@ -33,7 +33,7 @@
 class SmsProofPossessionController extends AbstractController
 {
     public function __construct(
-        private readonly SmsSecondFactorService $smsSecondFactorService,
+        private readonly SmsSecondFactorServiceInterface $smsSecondFactorService,
         private readonly ControllerCheckerService $checkerService,
     ) {
     }

src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/Registration/Sms/SmsSendChallengeController.php

@@ -23,7 +23,6 @@
 use Surfnet\StepupSelfService\SelfServiceBundle\Command\SendSmsChallengeCommand;
 use Surfnet\StepupSelfService\SelfServiceBundle\Form\Type\SendSmsChallengeType;
 use Surfnet\StepupSelfService\SelfServiceBundle\Service\ControllerCheckerService;
-use Surfnet\StepupSelfService\SelfServiceBundle\Service\SmsSecondFactorService;
 use Surfnet\StepupSelfService\SelfServiceBundle\Service\SmsSecondFactorServiceInterface;
 use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
 use Symfony\Component\HttpFoundation\Request;
@@ -33,7 +32,7 @@
 class SmsSendChallengeController extends AbstractController
 {
     public function __construct(
-        private readonly SmsSecondFactorService $smsSecondFactorService,
+        private readonly SmsSecondFactorServiceInterface $smsSecondFactorService,
         private readonly ControllerCheckerService $checkerService,
     ) {
     }

src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/Registration/YubikeyController.php

@@ -24,6 +24,7 @@
 use Surfnet\StepupSelfService\SelfServiceBundle\Command\VerifyYubikeyOtpCommand;
 use Surfnet\StepupSelfService\SelfServiceBundle\Form\Type\ProveYubikeyPossessionType;
 use Surfnet\StepupSelfService\SelfServiceBundle\Service\YubikeySecondFactorService;
+use Surfnet\StepupSelfService\SelfServiceBundle\Service\YubikeySecondFactorServiceInterface;
 use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
@@ -33,7 +34,7 @@ class YubikeyController extends AbstractController
 {
     public function __construct(
         private readonly ControllerCheckerService   $checkerService,
-        private readonly YubikeySecondFactorService $yubikeySecondFactorService,
+        private readonly YubikeySecondFactorServiceInterface $yubikeySecondFactorService,
     ) {
     }
 

src/Surfnet/StepupSelfService/SelfServiceBundle/Service/YubikeySecondFactorService.php

@@ -28,10 +28,10 @@
 use Surfnet\StepupSelfService\SelfServiceBundle\Exception\RuntimeException;
 use Surfnet\StepupSelfService\SelfServiceBundle\Service\YubikeySecondFactor\ProofOfPossessionResult;
 
-class YubikeySecondFactorService
+class YubikeySecondFactorService implements YubikeySecondFactorServiceInterface
 {
     public function __construct(
-        private readonly YubikeyService $yubikeyService,
+        private readonly YubikeyServiceInterface $yubikeyService,
         private readonly CommandService $commandService,
     ) {
     }

src/Surfnet/StepupSelfService/SelfServiceBundle/Service/YubikeySecondFactorServiceInterface.php

@@ -0,0 +1,30 @@
+<?php
+
+/**
+ * Copyright 2024 SURFnet bv
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+namespace Surfnet\StepupSelfService\SelfServiceBundle\Service;
+
+use Surfnet\StepupSelfService\SelfServiceBundle\Command\VerifyYubikeyOtpCommand;
+use Surfnet\StepupSelfService\SelfServiceBundle\Service\YubikeySecondFactor\ProofOfPossessionResult;
+
+interface YubikeySecondFactorServiceInterface
+{
+    /**
+     * Verifies the OTP result status
+     */
+    public function provePossession(VerifyYubikeyOtpCommand $command): ProofOfPossessionResult;
+}

src/Surfnet/StepupSelfService/SelfServiceBundle/Service/YubikeyService.php

@@ -26,7 +26,7 @@
 use Surfnet\StepupSelfService\SelfServiceBundle\Command\VerifyYubikeyOtpCommand;
 use RuntimeException;
 
-class YubikeyService
+class YubikeyService implements YubikeyServiceInterface
 {
     /**
      * @param Client $guzzleClient A Guzzle client configured with the Yubikey API base URL and authentication.

src/Surfnet/StepupSelfService/SelfServiceBundle/Service/YubikeyServiceInterface.php

@@ -0,0 +1,29 @@
+<?php
+
+/**
+ * Copyright 2024 SURFnet bv
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+namespace Surfnet\StepupSelfService\SelfServiceBundle\Service;
+
+use Surfnet\StepupSelfService\SelfServiceBundle\Command\VerifyYubikeyOtpCommand;
+
+interface YubikeyServiceInterface
+{
+    /**
+     * Verifies the OTP result status
+     */
+    public function verify(VerifyYubikeyOtpCommand $command): YubikeyVerificationResult;
+}

src/Surfnet/StepupSelfService/SelfServiceBundle/Tests/TestDouble/Service/SmsSecondFactorService.php

@@ -18,7 +18,6 @@
 
 namespace Surfnet\StepupSelfService\SelfServiceBundle\Tests\TestDouble\Service;
 
-use Surfnet\StepupBundle\Service\Exception\TooManyChallengesRequestedException;
 use Surfnet\StepupBundle\Service\SmsSecondFactor\OtpVerification;
 use Surfnet\StepupMiddlewareClientBundle\Identity\Command\ProvePhonePossessionCommand;
 use Surfnet\StepupMiddlewareClientBundle\Uuid\Uuid;
@@ -32,7 +31,6 @@
  * A test stand in for the SmsSecondFactorService
  *
  * This class should only be used in test context!
- *
  */
 class SmsSecondFactorService implements SmsSecondFactorServiceInterface
 {
@@ -44,25 +42,16 @@ public function __construct(private readonly CommandService $commandService)
     {
     }
 
-    /**
-     * @return int
-     */
     public function getOtpRequestsRemainingCount($identifier): int
     {
         return $this->maxOtpRequestRemaining;
     }
 
-    /**
-     * @return int
-     */
     public function getMaximumOtpRequestsCount(): int
     {
         return $this->maxOtpRequestCount;
     }
 
-    /**
-     * @return bool
-     */
     public function hasSmsVerificationState(string $secondFactorId): bool
     {
         return $this->verificationState;
@@ -74,11 +63,7 @@ public function clearSmsVerificationState(string $secondFactorId): bool
     }
 
     /**
-     * Always returns true, indicating sending did not fail.
-     *
-     * @param SendSmsChallengeCommand $command
-     * @return bool Whether SMS sending did not fail.
-     * @throws TooManyChallengesRequestedException
+     * Always returns true, indicating sending did not fail
      */
     public function sendChallenge(SendSmsChallengeCommand $command): bool
     {
@@ -87,14 +72,8 @@ public function sendChallenge(SendSmsChallengeCommand $command): bool
         return true;
     }
 
-    /**
-     *
-     * @param VerifySmsChallengeCommand $challengeCommand
-     * @return ProofOfPossessionResult
-     */
-    public function provePossession(VerifySmsChallengeCommand $challengeCommand): \Surfnet\StepupSelfService\SelfServiceBundle\Service\SmsSecondFactor\ProofOfPossessionResult
+    public function provePossession(VerifySmsChallengeCommand $challengeCommand): ProofOfPossessionResult
     {
-
         OtpVerification::foundMatch($challengeCommand->identity);
 
         $command = new ProvePhonePossessionCommand();

src/Surfnet/StepupSelfService/SelfServiceBundle/Tests/TestDouble/Service/YubikeySecondFactorService.php

@@ -0,0 +1,55 @@
+<?php
+
+/**
+ * Copyright 2024 SURFnet bv
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+namespace Surfnet\StepupSelfService\SelfServiceBundle\Tests\TestDouble\Service;
+
+use Psr\Log\LoggerInterface;
+use Surfnet\StepupMiddlewareClientBundle\Identity\Command\ProveYubikeyPossessionCommand;
+use Surfnet\StepupMiddlewareClientBundle\Uuid\Uuid;
+use Surfnet\StepupSelfService\SelfServiceBundle\Service\CommandService;
+use Surfnet\StepupSelfService\SelfServiceBundle\Command\VerifyYubikeyOtpCommand;
+use Surfnet\StepupSelfService\SelfServiceBundle\Service\YubikeySecondFactor\ProofOfPossessionResult;
+use Surfnet\StepupSelfService\SelfServiceBundle\Service\YubikeySecondFactorServiceInterface;
+
+
+/**
+ * Serves a test double for : ApiBundle/Service/YubikeyService
+ *
+ * This service will accept any OtpDto that it is fed, always returning a OtpVerificationResult with status STATUS_OK
+ */
+class YubikeySecondFactorService implements YubikeySecondFactorServiceInterface
+{
+
+    public function __construct(
+        private readonly CommandService $commandService,
+        private readonly LoggerInterface $logger)
+    {
+    }
+
+    public function provePossession(VerifyYubikeyOtpCommand $command): ProofOfPossessionResult
+    {
+        $this->logger->info('Using the Fake Yubikey SF service. This always returns a successful response.');
+        $provePossessionCommand = new ProveYubikeyPossessionCommand();
+        $provePossessionCommand->identityId = $command->identity;
+        $provePossessionCommand->secondFactorId = Uuid::generate();
+        $provePossessionCommand->yubikeyPublicId = '09999999';
+        $this->commandService->execute($provePossessionCommand);
+
+        return ProofOfPossessionResult::secondFactorCreated($provePossessionCommand->secondFactorId);
+    }
+}