Skip to content

Commit

Permalink
WIP
Browse files Browse the repository at this point in the history
  • Loading branch information
parijke authored and MKodde committed Dec 20, 2023
1 parent 5061e06 commit d448d5e
Show file tree
Hide file tree
Showing 15 changed files with 158 additions and 120 deletions.
File renamed without changes.
3 changes: 2 additions & 1 deletion ci/qa/phpstan.neon
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
includes:
- ./phpstan-baseline.neon
parameters:
level: 9
reportUnmatchedIgnoredErrors: false
level: 5
paths:
- ../../src
excludePaths:
Expand Down
5 changes: 5 additions & 0 deletions ci/qa/rector
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/usr/bin/env bash

cd $(dirname $0)/../../

vendor/bin/rector -n --memory-limit=-1 --no-ansi -c
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"ext-json": "*",
"ext-mbstring": "*",
"ext-openssl": "*",
"ext-dom": "*",
"doctrine/annotations": "^2.0",
"guzzlehttp/guzzle": "^7",
"incenteev/composer-parameter-handler": "~2.0",
Expand Down
174 changes: 92 additions & 82 deletions composer.lock

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions config/packages/framework.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# see https://symfony.com/doc/current/reference/configuration/framework.html
framework:
default_locale: 'en_GB'
translator:
default_path: '%kernel.project_dir%/translations'
fallbacks:
- 'en_GB'
esi: false
secret: "%secret%"
form: ~
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,10 @@ public function process(ContainerBuilder $container): void
// $container
// ->getDefinition('session')
// ->addMethodCall('registerBag', [new Reference('gssp.session.namespaced_attribute_bag')]);
// $container
// ->getDefinition('request_stack')
// ->addMethodCall('getSession', [])
// ->addMethodCall('registerBag', [new Reference('gssp.session.namespaced_attribute_bag')]);

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,11 @@ private function loadProviderConfiguration(
$stateHandlerDefinition = new Definition(
StateHandler::class,
[
new Reference('gssp.sessionbag'),
new Reference('request_stack'),
$provider
]
);

$container->setDefinition('gssp.provider.' . $provider . '.statehandler', $stateHandlerDefinition);

$providerDefinition = new Definition(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,15 @@ services:
Surfnet\StepupSelfService\SamlStepupProviderBundle\Provider\ProviderRepository:
alias: gssp.provider_repository

gssp.sessionbag:
class: Symfony\Component\HttpFoundation\Session\Attribute\AttributeBag
# factory: ['@request_stack', 'getSession']
arguments: ['gssp']
# gssp.sessionbag:
# class: Symfony\Component\HttpFoundation\Session\Attribute\AttributeBag

# gssp.session.attribute_bag:
# public: false
# class: Symfony\Component\HttpFoundation\Session\Attribute\AttributeBag
# arguments:
# - '__gssp__'
# - '/'
# calls:
# - [setName, ['gssp']]

gssp.session.attribute_bag:
public: false
class: Symfony\Component\HttpFoundation\Session\Attribute\AttributeBag
arguments:
- '__gssp__'
- '/'
calls:
- [setName, ['gssp']]
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@

namespace Surfnet\StepupSelfService\SamlStepupProviderBundle\Saml;

use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\HttpFoundation\Session\Attribute\AttributeBagInterface;

final readonly class StateHandler
final class StateHandler
{
const REQUEST_ID = 'request_id';

public function __construct(
private AttributeBagInterface $attributeBag,
private RequestStack $requestStack,
private string $provider,
) {
}
Expand All @@ -46,16 +47,25 @@ public function getRequestId(): ?string

public function clear(): void
{
$this->attributeBag->remove($this->provider);
// @todo this used to only remove the GSSP state for a provider. But now removes noting as $this->provider is not a session key
// The namespaced sesion bag would remove all session state prepended with $this->provider.
//
//demogssp/sstateitem1 => foobar
//demogssp/requestid => foobar
$session = $this->requestStack->getSession();
$session->getBag()->clear();

$this->requestStack->getSession()->remove($this->provider);
}

protected function set(string $key, $value): void
{
$this->attributeBag->set($this->provider . '/' . $key, $value);

$this->requestStack->getSession()->set($this->provider . '/' . $key, $value);
}

protected function get(string $key)
{
return $this->attributeBag->get($this->provider . '/' . $key);
return $this->requestStack->getSession()->get($this->provider . '/' . $key);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,14 @@ public function switchLocale(Request $request): RedirectResponse
return $this->redirect($returnUrl);
}


if (!$this->identityService->switchLocale($command)) {
$this->addFlash('error', $this->translator->trans('ss.flash.error_while_switching_locale'));
$this->logger->error('An error occurred while switching locales');
return $this->redirect($returnUrl);
}

$this->logger->info('Successfully switched locale');
$request->setLocale('nl_NL');

return $this->redirect($returnUrl);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ public function registrationEmailSent($secondFactorId): Response
/**
* @param $secondFactorId
* @return Response
* TODO: pdf generation in dedicated service
*/
#[Route(
path: '/registration/{secondFactorId}/registration-pdf',
Expand Down Expand Up @@ -284,7 +285,7 @@ public function registrationPdf($secondFactorId): Response
return $response;
}

private function buildRegistrationActionParameters($secondFactorId): array
private function buildRegistrationActionParameters(string $secondFactorId): array
{
$identity = $this->getIdentity();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

namespace Surfnet\StepupSelfService\SelfServiceBundle\Controller;

use LogicException;
use Surfnet\StepupBundle\DateTime\RegistrationExpirationHelper;
use Psr\Log\LoggerInterface;
use Surfnet\StepupBundle\Service\SecondFactorTypeService;
Expand All @@ -39,7 +40,7 @@
class SecondFactorController extends Controller
{
public function __construct(
LoggerInterface $logger,
private readonly LoggerInterface $logger,
private readonly InstitutionConfigurationOptionsService $configurationOptionsService,
private readonly RecoveryTokenService $recoveryTokenService,
private readonly AuthorizationService $authorizationService,
Expand Down Expand Up @@ -108,10 +109,8 @@ public function revoke(Request $request, string $state, string $secondFactorId):
{
$identity = $this->getIdentity();

/** @var SecondFactorService $service */
$service = $this->container->get('surfnet_stepup_self_service_self_service.service.second_factor');
if (!$service->identityHasSecondFactorOfStateWithId($identity->id, $state, $secondFactorId)) {
$this->container->get('logger')->error(sprintf(
if (!$this->secondFactorService->identityHasSecondFactorOfStateWithId($identity->id, $state, $secondFactorId)) {
$this->logger->error(sprintf(
'Identity "%s" tried to revoke "%s" second factor "%s", but does not own that second factor',
$identity->id,
$state,
Expand All @@ -121,9 +120,9 @@ public function revoke(Request $request, string $state, string $secondFactorId):
}

$secondFactor = match ($state) {
'unverified' => $service->findOneUnverified($secondFactorId),
'verified' => $service->findOneVerified($secondFactorId),
'vetted' => $service->findOneVetted($secondFactorId),
'unverified' => $this->secondFactorService->findOneUnverified($secondFactorId),
'verified' => $this->secondFactorService->findOneVerified($secondFactorId),
'vetted' => $this->secondFactorService->findOneVetted($secondFactorId),
default => throw new LogicException('There are no other types of second factor.'),
};

Expand All @@ -140,13 +139,11 @@ public function revoke(Request $request, string $state, string $secondFactorId):
$form = $this->createForm(RevokeSecondFactorType::class, $command)->handleRequest($request);

if ($form->isSubmitted() && $form->isValid()) {
/** @var FlashBagInterface $flashBag */
$flashBag = $this->container->get('session')->getFlashBag();

if ($service->revoke($command)) {
$flashBag->add('success', 'ss.second_factor.revoke.alert.revocation_successful');
if ($this->secondFactorService->revoke($command)) {
$this->addFlash('success', 'ss.second_factor.revoke.alert.revocation_successful');
} else {
$flashBag->add('error', 'ss.second_factor.revoke.alert.revocation_failed');
$this->addFlash('error', 'ss.second_factor.revoke.alert.revocation_failed');
}

return $this->redirectToRoute('ss_second_factor_list');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ public function selfAssertedTokenRegistrationRecoveryToken(
}
$command = new SafeStoreAuthenticationCommand();
$command->recoveryToken = $token;
$command->identity = $identity;
$command->identity = $identity->id;
$form = $this->createForm(AuthenticateSafeStoreType::class, $command)->handleRequest($request);
if ($form->isSubmitted() && $form->isValid()) {
if ($this->recoveryTokenService->authenticateSafeStore($command)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,11 @@ final class GlobalViewParameters
*/
private array $supportUrl;

public function __construct(private readonly TranslatorInterface $translator, array $locales, array $supportUrl)
{
public function __construct(
private readonly TranslatorInterface $translator,
array $locales,
array $supportUrl,
) {
Assert::keysAre($supportUrl, $locales);
$this->locales = $locales;
$this->supportUrl = $supportUrl;
Expand All @@ -45,7 +48,7 @@ public function __construct(private readonly TranslatorInterface $translator, ar
/**
* @return string
*/
public function getSupportUrl()
public function getSupportUrl(): string
{
return $this->supportUrl[$this->translator->getLocale()];
}
Expand Down

0 comments on commit d448d5e

Please sign in to comment.