From 72c9aab828395181d6564f5f270ef3021933fd55 Mon Sep 17 00:00:00 2001 From: Alexandre Lemaire Date: Wed, 3 Mar 2021 13:23:48 -0500 Subject: [PATCH] Samesite cookie policies now available. --- .../Service/AuthenticationServiceSpec.php | 20 +++--- config/circlical.user.local.php.dist | 4 ++ .../Controller/CliController.php | 1 - .../Service/AuthenticationServiceFactory.php | 3 +- .../Service/AuthenticationService.php | 69 ++++++++----------- 5 files changed, 48 insertions(+), 49 deletions(-) diff --git a/bundle/Spec/CirclicalUser/Service/AuthenticationServiceSpec.php b/bundle/Spec/CirclicalUser/Service/AuthenticationServiceSpec.php index 5fe8533..13d4b62 100644 --- a/bundle/Spec/CirclicalUser/Service/AuthenticationServiceSpec.php +++ b/bundle/Spec/CirclicalUser/Service/AuthenticationServiceSpec.php @@ -74,7 +74,8 @@ public function let(AuthenticationMapper $authenticationMapper, UserMapper $user false, new PasswordNotChecked(), true, - true + true, + 'None' ); } @@ -321,7 +322,6 @@ public function it_fails_when_any_cookies_are_missing() foreach ($results as $combinations) { $comboCount = count($combinations); if ($comboCount != 0 && $comboCount < 4) { - foreach ($cookieTypes as $c) { unset($_COOKIE[$c]); } @@ -418,7 +418,6 @@ public function it_wont_overwrite_existing_auth_on_create($authenticationMapper, public function it_wont_create_auth_when_email_usernames_belong_to_user_records($authenticationMapper, User $user5) { - $user5->getId()->willReturn(5); $user5->getEmail()->willReturn('alex@circlical.com'); $this->shouldThrow(EmailUsernameTakenException::class)->during('create', [$user5, 'alex@circlical.com', 'pepperspray']); @@ -455,7 +454,8 @@ public function it_will_create_new_auth_records_with_strong_passwords($authentic false, new Passwdqc(), true, - true + true, + 'None' ); $newAuth->getRawSessionKey()->willReturn(KeyFactory::generateEncryptionKey()->getRawKeyMaterial()); @@ -479,7 +479,8 @@ public function it_wont_create_new_auth_records_with_weak_passwords($authenticat false, new Passwdqc(), true, - true + true, + 'None' ); $newAuth->getRawSessionKey()->willReturn(KeyFactory::generateEncryptionKey()->getRawKeyMaterial()); @@ -505,7 +506,8 @@ public function it_wont_create_new_auth_records_with_weak_passwords_via_zxcvbn( false, new Zxcvbn([]), true, - true + true, + 'None' ); // Required, since the array-cast can't support closures which are passed by phpspec, even with getWrappedObject() @@ -563,7 +565,8 @@ public function it_fails_to_create_tokens_when_password_changes_are_prohibited($ false, new PasswordNotChecked(), true, - true + true, + 'None' ); $this->shouldThrow(PasswordResetProhibitedException::class)->during('createRecoveryToken', [$user]); } @@ -579,7 +582,8 @@ public function it_bails_on_password_changes_if_no_provider_is_set($authenticati false, new PasswordNotChecked(), true, - true + true, + 'None' ); $this->shouldThrow(PasswordResetProhibitedException::class)->during('changePasswordWithRecoveryToken', [$user, 123, 'string', 'string']); } diff --git a/config/circlical.user.local.php.dist b/config/circlical.user.local.php.dist index e64c370..b82e270 100644 --- a/config/circlical.user.local.php.dist +++ b/config/circlical.user.local.php.dist @@ -75,6 +75,10 @@ return [ */ 'secure_cookies' => false, + /* + * What SameSite policy do we give cookies? Can be set to null 'Strict, 'Lax' or 'None' + */ + 'samesite_policy' => 'None', /* * Forgot password functionality diff --git a/src/CirclicalUser/Controller/CliController.php b/src/CirclicalUser/Controller/CliController.php index 2721191..7d86abc 100644 --- a/src/CirclicalUser/Controller/CliController.php +++ b/src/CirclicalUser/Controller/CliController.php @@ -3,7 +3,6 @@ namespace CirclicalUser\Controller; use CirclicalUser\Entity\TemporaryResource; -use CirclicalUser\Mapper\UserMapper; use CirclicalUser\Provider\GroupPermissionProviderInterface; use CirclicalUser\Provider\RoleProviderInterface; use CirclicalUser\Provider\UserPermissionProviderInterface; diff --git a/src/CirclicalUser/Factory/Service/AuthenticationServiceFactory.php b/src/CirclicalUser/Factory/Service/AuthenticationServiceFactory.php index f70b4f6..719e1e1 100644 --- a/src/CirclicalUser/Factory/Service/AuthenticationServiceFactory.php +++ b/src/CirclicalUser/Factory/Service/AuthenticationServiceFactory.php @@ -54,7 +54,8 @@ public function __invoke(ContainerInterface $container, $requestedName, array $o $secure, $container->get(PasswordCheckerInterface::class), $userConfig['password_reset_tokens']['validate_fingerprint'] ?? true, - $userConfig['password_reset_tokens']['validate_ip'] ?? false + $userConfig['password_reset_tokens']['validate_ip'] ?? false, + $userConfig['auth']['samesite_policy'] ?? 'None' ); } } diff --git a/src/CirclicalUser/Service/AuthenticationService.php b/src/CirclicalUser/Service/AuthenticationService.php index 0cf6013..6a5aa27 100644 --- a/src/CirclicalUser/Service/AuthenticationService.php +++ b/src/CirclicalUser/Service/AuthenticationService.php @@ -52,7 +52,6 @@ class AuthenticationService */ public const COOKIE_VERIFY_B = '_sessionc'; - /** * Prefix for hash cookies, mmm. */ @@ -60,61 +59,46 @@ class AuthenticationService /** * Stores the user identity after having been authenticated. - * - * @var ?User */ - private $identity; + private ?User $identity; - /** - * @var AuthenticationProviderInterface - */ - private $authenticationProvider; + private AuthenticationProviderInterface $authenticationProvider; - /** - * @var UserProviderInterface - */ - private $userProvider; + private UserProviderInterface $userProvider; /** - * @var HiddenString A config-defined key that's used to encrypt ID cookie + * A config-defined key that's used to encrypt ID cookie */ - private $systemEncryptionKey; + private HiddenString $systemEncryptionKey; /** - * @var bool Should the cookie expire at the end of the session? + * Should the cookie expire at the end of the session? */ - private $transient; + private bool $transient; /** - * @var bool Should the cookie be marked as https only? + * Should the cookie be marked as secure? */ - private $secure; + private bool $secure; + private PasswordCheckerInterface $passwordChecker; - /** - * @var PasswordCheckerInterface - */ - private $passwordChecker; - + private ?UserResetTokenProviderInterface $resetTokenProvider; /** - * @var ?UserResetTokenProviderInterface + * If password reset is enabled, do we validate the browser fingerprint? */ - private $resetTokenProvider; - + private bool $validateFingerprint; /** - * @var boolean - * If password reset is enabled, do we validate the browser fingerprint? + * If password reset is enabled, do we validate the user IP address? */ - private $validateFingerprint; - + private bool $validateIp; /** - * @var boolean - * If password reset is enabled, do we validate the user IP address? + * Samesite cookie attribute */ - private $validateIp; + private string $sameSite; /** @@ -129,6 +113,7 @@ class AuthenticationService * @param PasswordCheckerInterface $passwordChecker Optional, a password checker implementation * @param bool $validateFingerprint If password reset is enabled, do we validate the browser fingerprint? * @param bool $validateIp If password reset is enabled, do we validate the user IP address? + * @param string $sameSite Should be one of 'None', 'Lax' or 'Strict'. */ public function __construct( AuthenticationProviderInterface $authenticationProvider, @@ -139,8 +124,10 @@ public function __construct( bool $secure, PasswordCheckerInterface $passwordChecker, bool $validateFingerprint, - bool $validateIp + bool $validateIp, + string $sameSite ) { + $this->identity = null; $this->authenticationProvider = $authenticationProvider; $this->userProvider = $userProvider; $this->systemEncryptionKey = new HiddenString($systemEncryptionKey); @@ -150,6 +137,7 @@ public function __construct( $this->resetTokenProvider = $resetTokenProvider; $this->validateFingerprint = $validateFingerprint; $this->validateIp = $validateIp; + $this->sameSite = $sameSite; } /** @@ -331,11 +319,14 @@ private function setCookie(string $name, string $value) setcookie( $name, $value, - $expiry, - '/', - $sessionParameters['domain'], - $this->secure, - true + [ + 'expires' => $expiry, + 'path' => '/', + 'domain' => $sessionParameters['domain'], + 'secure' => $this->secure, + 'httponly' => true, + 'samesite' => $this->sameSite, + ] ); }