Skip to content

Commit

Permalink
Merge branch '4.3' into 4.4
Browse files Browse the repository at this point in the history
* 4.3:
  fix translation domain
  tag the FileType service as a form type
  don't validate IP addresses from env var placeholders
  [Validator] Fix GroupSequenceProvider annotation
  [Messenger] fix delay exchange recreation after disconnect
  Update ajax security cheat sheet link
  Fix AuthenticationException::getToken typehint
  • Loading branch information
xabbuh committed Jun 21, 2019
2 parents dca9325 + a25c2af commit 431a769
Show file tree
Hide file tree
Showing 13 changed files with 61 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
<argument type="service" id="form.choice_list_factory"/>
</service>
<service id="form.type.file" class="Symfony\Component\Form\Extension\Core\Type\FileType" public="true">
<tag name="form.type" />
<argument type="service" id="translator" on-invalid="ignore" />
</service>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,15 +143,6 @@ private function addAccessControlSection(ArrayNodeDefinition $rootNode)
->integerNode('port')->defaultNull()->end()
->arrayNode('ips')
->beforeNormalization()->ifString()->then(function ($v) { return [$v]; })->end()
->beforeNormalization()->always()->then(function ($v) {
foreach ($v as $ip) {
if (false === $this->isValidIp($ip)) {
throw new \LogicException(sprintf('The given "%s" value in the "access_control" config option is not a valid IP address.', $ip));
}
}

return $v;
})->end()
->prototype('scalar')->end()
->end()
->arrayNode('methods')
Expand Down Expand Up @@ -432,30 +423,4 @@ private function addEncodersSection(ArrayNodeDefinition $rootNode)
->end()
;
}

private function isValidIp(string $cidr): bool
{
$cidrParts = explode('/', $cidr);

if (1 === \count($cidrParts)) {
return false !== filter_var($cidrParts[0], FILTER_VALIDATE_IP);
}

$ip = $cidrParts[0];
$netmask = $cidrParts[1];

if (!ctype_digit($netmask)) {
return false;
}

if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
return $netmask <= 32;
}

if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) {
return $netmask <= 128;
}

return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -731,20 +731,32 @@ private function createExpression($container, $expression)
return $this->expressions[$id] = new Reference($id);
}

private function createRequestMatcher($container, $path = null, $host = null, int $port = null, $methods = [], $ip = null, array $attributes = [])
private function createRequestMatcher(ContainerBuilder $container, $path = null, $host = null, int $port = null, $methods = [], array $ips = null, array $attributes = [])
{
if ($methods) {
$methods = array_map('strtoupper', (array) $methods);
}

$id = '.security.request_matcher.'.ContainerBuilder::hash([$path, $host, $port, $methods, $ip, $attributes]);
if (null !== $ips) {
foreach ($ips as $ip) {
$container->resolveEnvPlaceholders($ip, null, $usedEnvs);

if (!$usedEnvs && !$this->isValidIp($ip)) {
throw new \LogicException(sprintf('The given value "%s" in the "security.access_control" config option is not a valid IP address.', $ip));
}

$usedEnvs = null;
}
}

$id = '.security.request_matcher.'.ContainerBuilder::hash([$path, $host, $port, $methods, $ips, $attributes]);

if (isset($this->requestMatchers[$id])) {
return $this->requestMatchers[$id];
}

// only add arguments that are necessary
$arguments = [$path, $host, $methods, $ip, $attributes, null, $port];
$arguments = [$path, $host, $methods, $ips, $attributes, null, $port];
while (\count($arguments) > 0 && !end($arguments)) {
array_pop($arguments);
}
Expand Down Expand Up @@ -788,4 +800,30 @@ public function getConfiguration(array $config, ContainerBuilder $container)
// first assemble the factories
return new MainConfiguration($this->factories, $this->userProviderFactories);
}

private function isValidIp(string $cidr): bool
{
$cidrParts = explode('/', $cidr);

if (1 === \count($cidrParts)) {
return false !== filter_var($cidrParts[0], FILTER_VALIDATE_IP);
}

$ip = $cidrParts[0];
$netmask = $cidrParts[1];

if (!ctype_digit($netmask)) {
return false;
}

if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
return $netmask <= 32;
}

if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) {
return $netmask <= 128;
}

return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ secured-by-one-real-ip-with-mask:
secured-by-one-real-ipv6:
path: /secured-by-one-real-ipv6

secured-by-one-env-placeholder:
path: /secured-by-one-env-placeholder

secured-by-one-env-placeholder-and-one-real-ip:
path: /secured-by-one-env-placeholder-and-one-real-ip

form_logout:
path: /logout_path

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public function testSecurityConfigurationForExpression($config)
public function testInvalidIpsInAccessControl()
{
$this->expectException(\LogicException::class);
$this->expectExceptionMessage('The given "256.357.458.559" value in the "access_control" config option is not a valid IP address.');
$this->expectExceptionMessage('The given value "256.357.458.559" in the "security.access_control" config option is not a valid IP address.');

$client = $this->createClient(['test_case' => 'StandardFormLogin', 'root_config' => 'invalid_ip_access_control.yml']);
$client->request('GET', '/unprotected_resource');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
imports:
- { resource: ./../config/default.yml }

parameters:
env(APP_IP): '127.0.0.1'

security:
encoders:
Symfony\Component\Security\Core\User\User: plaintext
Expand Down Expand Up @@ -43,6 +46,8 @@ security:
- { path: ^/secured-by-one-real-ip$, ips: 198.51.100.0, roles: IS_AUTHENTICATED_ANONYMOUSLY }
- { path: ^/secured-by-one-real-ip-with-mask$, ips: '203.0.113.0/24', roles: IS_AUTHENTICATED_ANONYMOUSLY }
- { path: ^/secured-by-one-real-ipv6$, ips: 0:0:0:0:0:ffff:c633:6400, roles: IS_AUTHENTICATED_ANONYMOUSLY }
- { path: ^/secured-by-one-env-placeholder$, ips: '%env(APP_IP)%', roles: IS_AUTHENTICATED_ANONYMOUSLY }
- { path: ^/secured-by-one-env-placeholder-and-one-real-ip$, ips: ['%env(APP_IP)%', 198.51.100.0], roles: IS_AUTHENTICATED_ANONYMOUSLY }
- { path: ^/highly_protected_resource$, roles: IS_ADMIN }
- { path: ^/protected-via-expression$, allow_if: "(is_anonymous() and request.headers.get('user-agent') matches '/Firefox/i') or is_granted('ROLE_USER')" }
- { path: .*, roles: IS_AUTHENTICATED_FULLY }
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ security:

access_control:
# the '256.357.458.559' IP is wrong on purpose, to check invalid IP errors
- { path: ^/unprotected_resource$, ips: [1.1.1.1, 256.357.458.559], roles: IS_AUTHENTICATED_ANONYMOUSLY }
- { path: ^/unprotected_resource$, ips: [1.1.1.1, '%env(APP_IP)%', 256.357.458.559], roles: IS_AUTHENTICATED_ANONYMOUSLY }
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ private function getFileUploadError($errorCode)
}

if (null !== $this->translator) {
$message = $this->translator->trans($messageTemplate, $messageParameters);
$message = $this->translator->trans($messageTemplate, $messageParameters, 'validators');
} else {
$message = strtr($messageTemplate, $messageParameters);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpFoundation/JsonResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* object. It is however recommended that you do return an object as it
* protects yourself against XSSI and JSON-JavaScript Hijacking.
*
* @see https://www.owasp.org/index.php/OWASP_AJAX_Security_Guidelines#Always_return_JSON_with_an_Object_on_the_outside
* @see https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/AJAX_Security_Cheat_Sheet.md#always-return-json-with-an-object-on-the-outside
*
* @author Igor Wiedler <igor@wiedler.ch>
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,7 @@ private function clear(): void
$this->amqpChannel = null;
$this->amqpQueues = [];
$this->amqpExchange = null;
$this->amqpDelayExchange = null;
}

private function shouldSetup(): bool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class AuthenticationException extends RuntimeException
/**
* Get the token.
*
* @return TokenInterface
* @return TokenInterface|null
*/
public function getToken()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class GroupSequence
/**
* The groups in the sequence.
*
* @var string[]|array[]|GroupSequence[]
* @var string[]|string[][]|GroupSequence[]
*/
public $groups;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ interface GroupSequenceProviderInterface
* Returns which validation groups should be used for a certain state
* of the object.
*
* @return string[]|GroupSequence An array of validation groups
* @return string[]|string[][]|GroupSequence An array of validation groups
*/
public function getGroupSequence();
}

0 comments on commit 431a769

Please sign in to comment.