8000 [Security] Added type-hints to auth providers, tokens and voters by derrabus · Pull Request #32351 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Security] Added type-hints to auth providers, tokens and voters #32351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public function __construct(Connection $conn)
/**
* {@inheritdoc}
*/
public function loadTokenBySeries($series)
public function loadTokenBySeries(string $series)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 These changes to doctrine-bridge 5 break compatibility with security-core 4.4, thus the bump in composer.json. Can be reverted and re-applied later, if you don't want to bump yet.

{
// the alias for lastUsed works around case insensitivity in PostgreSQL
$sql = 'SELECT class, username, value, lastUsed AS last_used'
Expand All @@ -68,7 +68,7 @@ public function loadTokenBySeries($series)
/**
* {@inheritdoc}
*/
public function deleteTokenBySeries($series)
public function deleteTokenBySeries(string $series)
{
$sql = 'DELETE FROM rememberme_token WHERE series=:series';
$paramValues = ['series' => $series];
Expand All @@ -79,7 +79,7 @@ public function deleteTokenBySeries($series)
/**
* {@inheritdoc}
*/
public function updateToken($series, $tokenValue, \DateTime $lastUsed)
public function updateToken(string $series, string $tokenValue, \DateTime $lastUsed)
{
$sql = 'UPDATE rememberme_token SET value=:value, lastUsed=:lastUsed'
.' WHERE series=:series';
Expand Down
5 changes: 3 additions & 2 deletions src/Symfony/Bridge/Doctrine/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"symfony/property-access": "^4.4|^5.0",
"symfony/property-info": "^4.4|^5.0",
"symfony/proxy-manager-bridge": "^4.4|^5.0",
"symfony/security-core": "^4.4|^5.0",
"symfony/security-core": "^5.0",
"symfony/expression-language": "^4.4|^5.0",
"symfony/validator": "^4.4|^5.0",
"symfony/translation": "^4.4|^5.0",
Expand All @@ -49,7 +49,8 @@
"phpunit/phpunit": "<5.4.3",
"symfony/dependency-injection": "<4.4",
"symfony/form": "<4.4",
"symfony/messenger": "<4.4"
"symfony/messenger": "<4.4",
"symfony/security-core": "<5"
},
"suggest": {
"symfony/form": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ protected function checkAuthentication(UserInterface $user, UsernamePasswordToke
/**
* {@inheritdoc}
*/
protected function retrieveUser($username, UsernamePasswordToken $token)
protected function retrieveUser(string $username, UsernamePasswordToken $token)
{
$user = $token->getUser();
if ($user instanceof UserInterface) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,16 @@ public function __construct(UserProviderInterface $userProvider, UserCheckerInte

/**
* Set a query string to use in order to find a DN for the username.
*
* @param string $queryString
*/
public function setQueryString($queryString)
public function setQueryString(string $queryString)
{
$this->queryString = $queryString;
}

/**
* {@inheritdoc}
*/
protected function retrieveUser($username, UsernamePasswordToken $token)
protected function retrieveUser(string $username, UsernamePasswordToken $token)
{
if (AuthenticationProviderInterface::USERNAME_NONE_PROVIDED === $username) {
throw new UsernameNotFoundException('Username can not be null');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,11 @@ public function supports(TokenInterface $token)
/**
* Retrieves the user from an implementation-specific location.
*
* @param string $username The username to retrieve
* @param UsernamePasswordToken $token The Token
*
* @return UserInterface The user
*
* @throws AuthenticationException if the credentials could not be validated
*/
abstract protected function retrieveUser($username, UsernamePasswordToken $token);
abstract protected function retrieveUser(string $username, UsernamePasswordToken $token);

/**
* Does additional checks on the user and token (like validating the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class InMemoryTokenProvider implements TokenProviderInterface
/**
* {@inheritdoc}
*/
public function loadTokenBySeries($series)
public function loadTokenBySeries(string $series)
{
if (!isset($this->tokens[$series])) {
throw new TokenNotFoundException('No token found.');
Expand All @@ -37,7 +37,7 @@ public function loadTokenBySeries($series)
/**
* {@inheritdoc}
*/
public function updateToken($series, $tokenValue, \DateTime $lastUsed)
public function updateToken(string $series, string $tokenValue, \DateTime $lastUsed)
{
if (!isset($this->tokens[$series])) {
throw new TokenNotFoundException('No token found.');
Expand All @@ -56,7 +56,7 @@ public function updateToken($series, $tokenValue, \DateTime $lastUsed)
/**
* {@inheritdoc}
*/
public function deleteTokenBySeries($series)
public function deleteTokenBySeries(string $series)
{
unset($this->tokens[$series]);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,31 +23,23 @@ interface TokenProviderInterface
/**
* Loads the active token for the given series.
*
* @param string $series
*
* @return PersistentTokenInterface
*
* @throws TokenNotFoundException if the token is not found
*/
public function loadTokenBySeries($series);
public function loadTokenBySeries(string $series);

/**
* Deletes all tokens belonging to series.
*
* @param string $series
*/
public function deleteTokenBySeries($series);
public function deleteTokenBySeries(string $series);

/**
* Updates the token according to this data.
*
* @param string $series
* @param string $tokenValue
* @param \DateTime $lastUsed
*
* @throws TokenNotFoundException if the token is not found
*/
public function updateToken($series, $tokenValue, \DateTime $lastUsed);
public function updateToken(string $series, string $tokenValue, \DateTime $lastUsed);

/**
* Creates a new token.
Expand Down
10000
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,9 @@ public function isAuthenticated()
/**
* {@inheritdoc}
*/
public function setAuthenticated($authenticated)
public function setAuthenticated(bool $authenticated)
{
$this->authenticated = (bool) $authenticated;
$this->authenticated = $authenticated;
}

/**
Expand Down Expand Up @@ -187,25 +187,21 @@ public function setAttributes(array $attributes)
/**
* Returns true if the attribute exists.
*
* @param string $name The attribute name
*
* @return bool true if the attribute exists, false otherwise
*/
public function hasAttribute($name)
public function hasAttribute(string $name)
{
return \array_key_exists($name, $this->attributes);
}

/**
* Returns an attribute value.
*
* @param string $name The attribute name
*
* @return mixed The attribute value
*
* @throws \InvalidArgumentException When attribute doesn't exist for this token
*/
public function getAttribute($name)
public function getAttribute(string $name)
{
if (!\array_key_exists($name, $this->attributes)) {
throw new \InvalidArgumentException(sprintf('This token has no "%s" attribute.', $name));
Expand All @@ -217,10 +213,9 @@ public function getAttribute($name)
/**
* Sets an attribute.
*
* @param string $name The attribute name
* @param mixed $value The attribute value
* @param mixed $value The attribute value
*/
public function setAttribute($name, $value)
public function setAttribute(string $name, $value)
{
$this->attributes[$name] = $value;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public function __construct(UserInterface $user, string $providerKey, string $se
/**
* {@inheritdoc}
*/
public function setAuthenticated($authenticated)
public function setAuthenticated(bool $authenticated)
{
if ($authenticated) {
throw new \LogicException('You cannot set this token to authenticated after creation.');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,8 @@ public function isAuthenticated();

/**
* Sets the authenticated flag.
*
* @param bool $isAuthenticated The authenticated flag
*/
public function setAuthenticated($isAuthenticated);
public function setAuthenticated(bool $isAuthenticated);

/**
* Removes sensitive information from the token.
Expand All @@ -107,30 +105,25 @@ public function setAttributes(array $attributes);
/**
* Returns true if the attribute exists.
*
* @param string $name The attribute name
*
* @return bool true if the attribute exists, false otherwise
*/
public function hasAttribute($name);
public function hasAttribute(string $name);

/**
* Returns an attribute value.
*
* @param string $name The attribute name
*
* @return mixed The attribute value
*
* @throws \InvalidArgumentException When attribute doesn't exist for this token
*/
public function getAttribute($name);
public function getAttribute(string $name);

/**
* Sets an attribute.
*
* @param string $name The attribute name
* @param mixed $value The attribute value
* @param mixed $value The attribute value
*/
public function setAttribute($name, $value);
public function setAttribute(string $name, $value);

/**
* Returns all the necessary state of the object for serialization purposes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public function __construct($user, $credentials, string $providerKey, array $rol
/**
* {@inheritdoc}
*/
public function setAuthenticated($isAuthenticated)
public function setAuthenticated(bool $isAuthenticated)
{
if ($isAuthenticated) {
throw new \LogicException('Cannot set this token to trusted after instantiation.');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public function vote(TokenInterface $token, $subject, array $attributes)
*
* @return bool True if the attribute and subject are supported, false otherwise
*/
abstract protected function supports($attribute, $subject);
abstract protected function supports(string $attribute, $subject);

/**
* Perform a single access check operation on a given attribute, subject and token.
Expand All @@ -66,5 +66,5 @@ abstract protected function supports($attribute, $subject);
*
* @return bool
*/
abstract protected function voteOnAttribute($attribute, $subject, TokenInterface $token);
abstract protected function voteOnAttribute(string $attribute, $subject, TokenInterface $token);
}
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ public function isAuthenticated()
{
}

public function setAuthenticated($isAuthenticated)
public function setAuthenticated(bool $isAuthenticated)
{
}

Expand All @@ -175,15 +175,15 @@ public function setAttributes(array $attributes)
{
}

public function hasAttribute($name)
public function hasAttribute(string $name)
{
}

public function getAttribute($name)
public function getAttribute(string $name)
{
}

public function setAttribute($name, $value)
public function setAttribute(string $name, $value)
{
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public function tes 10000 tRetrieveUserReturnsUserFromTokenOnReauthentication()
$provider = new DaoAuthenticationProvider($userProvider, $this->getMockBuilder('Symfony\\Component\\Security\\Core\\User\\UserCheckerInterface')->getMock(), 'key', $this->getMockBuilder('Symfony\\Component\\Security\\Core\\Encoder\\EncoderFactoryInterface')->getMock());
$reflection = new \ReflectionMethod($provider, 'retrieveUser');
$reflection->setAccessible(true);
$result = $reflection->invoke($provider, null, $token);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 null shouldn't be a valid argument for the retrieveUser method. The test only worked because the parameter is ignored on the execution path that is tested here.

$result = $reflection->invoke($provider, 'someUser', $token);

$this->assertSame($user, $result);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,12 @@ public function testVote(array $attributes, $expectedVote, $object, $message)

class VoterTest_Voter extends Voter
{
protected function voteOnAttribute($attribute, $object, TokenInterface $token)
protected function voteOnAttribute(string $attribute, $object, TokenInterface $token)
{
return 'EDIT' === $attribute;
}

protected function supports($attribute, $object)
protected function supports(string $attribute, $object)
{
return $object instanceof \stdClass && \in_array($attribute, ['EDIT', 'CREATE']);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public function getCredentials()
return $this->credentials;
}

public function setAuthenticated($authenticated)
public function setAuthenticated(bool $authenticated)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 This change to security-guard 5 breaks compatibility with security-core 4.4, thus the bump in composer.json. Can be reverted and re-applied later, if you don't want to bump yet.

{
throw new \LogicException('The PreAuthenticationGuardToken is *never* authenticated.');
}
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Security/Guard/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
],
"require": {
"php": "^7.2.9",
"symfony/security-core": "^4.4|^5.0",
"symfony/security-core": "^5.0",
"symfony/security-http": "^4.4|^5.0"
},
"require-dev": {
Expand Down
0