Skip to content

Commit

Permalink
Merge pull request #63 from Saeven/feature/samesite-cookies
Browse files Browse the repository at this point in the history
Samesite cookie policies now available.
  • Loading branch information
Saeven authored Mar 3, 2021
2 parents 8178eb7 + 72c9aab commit 0cca038
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 49 deletions.
20 changes: 12 additions & 8 deletions bundle/Spec/CirclicalUser/Service/AuthenticationServiceSpec.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ public function let(AuthenticationMapper $authenticationMapper, UserMapper $user
false,
new PasswordNotChecked(),
true,
true
true,
'None'
);
}

Expand Down Expand Up @@ -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]);
}
Expand Down Expand Up @@ -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('[email protected]');
$this->shouldThrow(EmailUsernameTakenException::class)->during('create', [$user5, '[email protected]', 'pepperspray']);
Expand Down Expand Up @@ -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());
Expand All @@ -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());
Expand All @@ -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()
Expand Down Expand Up @@ -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]);
}
Expand All @@ -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']);
}
Expand Down
4 changes: 4 additions & 0 deletions config/circlical.user.local.php.dist
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion src/CirclicalUser/Controller/CliController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
);
}
}
69 changes: 30 additions & 39 deletions src/CirclicalUser/Service/AuthenticationService.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,69 +52,53 @@ class AuthenticationService
*/
public const COOKIE_VERIFY_B = '_sessionc';


/**
* Prefix for hash cookies, mmm.
*/
public const COOKIE_HASH_PREFIX = '__cu';

/**
* 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;


/**
Expand All @@ -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,
Expand All @@ -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);
Expand All @@ -150,6 +137,7 @@ public function __construct(
$this->resetTokenProvider = $resetTokenProvider;
$this->validateFingerprint = $validateFingerprint;
$this->validateIp = $validateIp;
$this->sameSite = $sameSite;
}

/**
Expand Down Expand Up @@ -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,
]
);
}

Expand Down

0 comments on commit 0cca038

Please sign in to comment.