Skip to content

Commit

Permalink
feature #28061 [Security] add port in access_control (roukmoute)
Browse files Browse the repository at this point in the history
This PR was squashed before being merged into the 4.2-dev branch (closes #28061).

Discussion
----------

[Security] add port in access_control

| Q             | A
| ------------- | ---
| Branch?       | master for features
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #26071
| License       | MIT
| Doc PR        | symfony/symfony-docs#10359

Add port in access_control

__Please Squash this P.R.__

Commits
-------

6413dcb [Security] add port in access_control
  • Loading branch information
fabpot committed Oct 10, 2018
2 parents dce8f08 + 6413dcb commit 331a24e
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 11 deletions.
1 change: 1 addition & 0 deletions src/Symfony/Bundle/SecurityBundle/CHANGELOG.md
Expand Up @@ -15,6 +15,7 @@ CHANGELOG
and added an "auto" mode to their "secure" config option to make them secure on HTTPS automatically.
* Deprecated the `simple_form` and `simple_preauth` authentication listeners, use Guard instead.
* Deprecated the `SimpleFormFactory` and `SimplePreAuthenticationFactory` classes, use Guard instead.
* Added `port` in access_control

4.1.0
-----
Expand Down
Expand Up @@ -142,6 +142,7 @@ private function addAccessControlSection(ArrayNodeDefinition $rootNode)
->example('^/path to resource/')
->end()
->scalarNode('host')->defaultNull()->end()
->integerNode('port')->defaultNull()->end()
->arrayNode('ips')
->beforeNormalization()->ifString()->then(function ($v) { return array($v); })->end()
->prototype('scalar')->end()
Expand Down
Expand Up @@ -172,6 +172,7 @@ private function createAuthorization($config, ContainerBuilder $container)
$container,
$access['path'],
$access['host'],
$access['port'],
$access['methods'],
$access['ips']
);
Expand Down Expand Up @@ -275,7 +276,7 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
$pattern = isset($firewall['pattern']) ? $firewall['pattern'] : null;
$host = isset($firewall['host']) ? $firewall['host'] : null;
$methods = isset($firewall['methods']) ? $firewall['methods'] : array();
$matcher = $this->createRequestMatcher($container, $pattern, $host, $methods);
$matcher = $this->createRequestMatcher($container, $pattern, $host, null, $methods);
}

$config->replaceArgument(2, $matcher ? (string) $matcher : null);
Expand Down Expand Up @@ -696,20 +697,20 @@ private function createExpression($container, $expression)
return $this->expressions[$id] = new Reference($id);
}

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

$id = '.security.request_matcher.'.ContainerBuilder::hash(array($path, $host, $methods, $ip, $attributes));
$id = '.security.request_matcher.'.ContainerBuilder::hash(array($path, $host, $port, $methods, $ip, $attributes));

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

// only add arguments that are necessary
$arguments = array($path, $host, $methods, $ip, $attributes);
$arguments = array($path, $host, $methods, $ip, $attributes, null, $port);
while (\count($arguments) > 0 && !end($arguments)) {
array_pop($arguments);
}
Expand Down
Expand Up @@ -84,7 +84,7 @@ public function testFirewalls()
array(
'simple',
'security.user_checker',
'.security.request_matcher.6tndozi',
'.security.request_matcher.xmi9dcw',
false,
),
array(
Expand Down Expand Up @@ -116,7 +116,7 @@ public function testFirewalls()
array(
'host',
'security.user_checker',
'.security.request_matcher.and0kk1',
'.security.request_matcher.iw4hyjb',
true,
false,
'security.user.provider.concrete.default',
Expand Down Expand Up @@ -238,7 +238,7 @@ public function testAccess()
$this->assertEquals(array('ROLE_USER'), $attributes);
$this->assertEquals('https', $channel);
$this->assertEquals(
array('/blog/524', null, array('GET', 'POST')),
array('/blog/524', null, array('GET', 'POST'), array(), array(), null, 8000),
$requestMatcher->getArguments()
);
} elseif (2 === $i) {
Expand Down
Expand Up @@ -90,7 +90,7 @@
),

'access_control' => array(
array('path' => '/blog/524', 'role' => 'ROLE_USER', 'requires_channel' => 'https', 'methods' => array('get', 'POST')),
array('path' => '/blog/524', 'role' => 'ROLE_USER', 'requires_channel' => 'https', 'methods' => array('get', 'POST'), 'port' => 8000),
array('path' => '/blog/.*', 'role' => 'IS_AUTHENTICATED_ANONYMOUSLY'),
array('path' => '/blog/524', 'role' => 'IS_AUTHENTICATED_ANONYMOUSLY', 'allow_if' => "token.getUsername() matches '/^admin/'"),
),
Expand Down
Expand Up @@ -72,7 +72,7 @@
<role id="ROLE_SUPER_ADMIN">ROLE_USER,ROLE_ADMIN,ROLE_ALLOWED_TO_SWITCH</role>
<role id="ROLE_REMOTE">ROLE_USER,ROLE_ADMIN</role>

<rule path="/blog/524" role="ROLE_USER" requires-channel="https" methods="get,POST" />
<rule path="/blog/524" role="ROLE_USER" requires-channel="https" methods="get,POST" port="8000" />
<rule role='IS_AUTHENTICATED_ANONYMOUSLY' path="/blog/.*" />
<rule role='IS_AUTHENTICATED_ANONYMOUSLY' allow-if="token.getUsername() matches '/^admin/'" path="/blog/524" />
</config>
Expand Down
Expand Up @@ -76,7 +76,7 @@ security:
ROLE_REMOTE: ROLE_USER,ROLE_ADMIN

access_control:
- { path: /blog/524, role: ROLE_USER, requires_channel: https, methods: [get, POST]}
- { path: /blog/524, role: ROLE_USER, requires_channel: https, methods: [get, POST], port: 8000}
-
path: /blog/.*
role: IS_AUTHENTICATED_ANONYMOUSLY
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/HttpFoundation/CHANGELOG.md
Expand Up @@ -8,6 +8,7 @@ CHANGELOG
* the default value of the "$secure" and "$samesite" arguments of Cookie's constructor
will respectively change from "false" to "null" and from "null" to "lax" in Symfony
5.0, you should define their values explicitly or use "Cookie::create()" instead.
* added `matchPort()` in RequestMatcher

4.1.3
-----
Expand Down
22 changes: 21 additions & 1 deletion src/Symfony/Component/HttpFoundation/RequestMatcher.php
Expand Up @@ -28,6 +28,11 @@ class RequestMatcher implements RequestMatcherInterface
*/
private $host;

/**
* @var int|null
*/
private $port;

/**
* @var string[]
*/
Expand Down Expand Up @@ -56,13 +61,14 @@ class RequestMatcher implements RequestMatcherInterface
* @param array $attributes
* @param string|string[]|null $schemes
*/
public function __construct(string $path = null, string $host = null, $methods = null, $ips = null, array $attributes = array(), $schemes = null)
public function __construct(string $path = null, string $host = null, $methods = null, $ips = null, array $attributes = array(), $schemes = null, int $port = null)
{
$this->matchPath($path);
$this->matchHost($host);
$this->matchMethod($methods);
$this->matchIps($ips);
$this->matchScheme($schemes);
$this->matchPort($port);

foreach ($attributes as $k => $v) {
$this->matchAttribute($k, $v);
Expand All @@ -89,6 +95,16 @@ public function matchHost($regexp)
$this->host = $regexp;
}

/**
* Adds a check for the the URL port.
*
* @param int|null $port The port number to connect to
*/
public function matchPort(int $port = null)
{
$this->port = $port;
}

/**
* Adds a check for the URL path info.
*
Expand Down Expand Up @@ -167,6 +183,10 @@ public function matches(Request $request)
return false;
}

if (null !== $this->port && 0 < $this->port && $request->getPort() !== $this->port) {
return false;
}

if (IpUtils::checkIp($request->getClientIp(), $this->ips)) {
return true;
}
Expand Down
15 changes: 15 additions & 0 deletions src/Symfony/Component/HttpFoundation/Tests/RequestMatcherTest.php
Expand Up @@ -78,6 +78,21 @@ public function testHost($pattern, $isMatch)
$this->assertSame($isMatch, $matcher->matches($request));
}

public function testPort()
{
$matcher = new RequestMatcher();
$request = Request::create('', 'get', array(), array(), array(), array('HTTP_HOST' => null, 'SERVER_PORT' => 8000));

$matcher->matchPort(8000);
$this->assertTrue($matcher->matches($request));

$matcher->matchPort(9000);
$this->assertFalse($matcher->matches($request));

$matcher = new RequestMatcher(null, null, null, null, array(), null, 8000);
$this->assertTrue($matcher->matches($request));
}

public function getHostData()
{
return array(
Expand Down

0 comments on commit 331a24e

Please sign in to comment.