Skip to content

[WIP][RFC] Signed internal uris #6463

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

Closed
wants to merge 4 commits into from
Closed
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
34 changes: 0 additions & 34 deletions UPGRADE-2.2.md
Original file line number Diff line number Diff line change
@@ -1,24 +1,6 @@
UPGRADE FROM 2.1 to 2.2
=======================

### TwigBridge

* The `render` tag signature and arguments changed.

Before:

```
{% render 'BlogBundle:Post:list' with { 'limit': 2 }, { 'alt': 'BlogBundle:Post:error' } %}
```

After:

```
{% render url('post_list', { 'limit': 2 }), { 'alt': 'BlogBundle:Post:error' } %}
```

where `post_list` is the route name for the `BlogBundle:Post:list` controller.

### HttpFoundation

* The MongoDbSessionHandler default field names and timestamp type have changed.
Expand Down Expand Up @@ -364,22 +346,6 @@

### FrameworkBundle

* The `render` method of the `actions` templating helper signature and arguments changed:

Before:

```
<?php echo $view['actions']->render('BlogBundle:Post:list', array('limit' => 2), array('alt' => 'BlogBundle:Post:error')) ?>
```

After:

```
<?php echo $view['actions']->render($view['router']->generate('post_list', array('limit' => 2)), array('alt' => 'BlogBundle:Post:error')) ?>
```

where `post_list` is the route name for the `BlogBundle:Post:list` controller.

#### Configuration

* The 2.2 version introduces a new parameter `trusted_proxies` that replaces
Expand Down
1 change: 0 additions & 1 deletion src/Symfony/Bridge/Twig/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ CHANGELOG
2.2.0
-----

* [BC BREAK] restricted the `render` tag to only accept URIs as reference (the signature changed)
* added a render function to render a request
* The `app` global variable is now injected even when using the twig service directly.

Expand Down
5 changes: 0 additions & 5 deletions src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@ CHANGELOG
2.2.0
-----

* [BC BREAK] restricted the `Symfony\Bundle\FrameworkBundle\HttpKernel::render()` method to only accept URIs as reference
* `Symfony\Bundle\FrameworkBundle\HttpKernel::render()` method signature changed and the first argument
must now be a URI (the `generateInternalUri()` method was removed)
* The internal routes have been removed (`Resources/config/routing/internal.xml`)
* The `render` method of the `actions` templating helper signature and arguments changed:
* replaced Symfony\Bundle\FrameworkBundle\Controller\TraceableControllerResolver by Symfony\Component\HttpKernel\Controller\TraceableControllerResolver
* replaced Symfony\Component\HttpKernel\Debug\ContainerAwareTraceableEventDispatcher by Symfony\Component\HttpKernel\Debug\TraceableEventDispatcher
* added Client::enableProfiler()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\FrameworkBundle\Controller;

use Symfony\Component\DependencyInjection\ContainerAware;
use Symfony\Component\HttpFoundation\Response;

/**
* InternalController.
*
* @author Fabien Potencier <fabien@symfony.com>
*/
class InternalController extends ContainerAware
{
/**
* Forwards to the given controller with the given path.
*
* @param string $path The path
* @param string $controller The controller name
*
* @return Response A Response instance
*/
public function indexAction($path, $controller)
{
// safeguard
if (!is_string($controller)) {
throw new \RuntimeException('A Controller must be a string.');
}

// check that the controller looks like a controller
if (false === strpos($controller, '::')) {
$count = substr_count($controller, ':');
if (2 == $count) {
// the convention already enforces the Controller suffix
} elseif (1 == $count) {
// controller in the service:method notation
list($service, $method) = explode(':', $controller, 2);
$class = get_class($this->container->get($service));

if (!preg_match('/Controller$/', $class)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is the convention, but why must it be a hard requirement? (Maybe it already is in full stack, not sure.)

Copy link
Member

Choose a reason for hiding this comment

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

Using direct controllers is not supported at all in 2.2. This change allows to limit the security issue by allowing to execute code only in controllers (as this feature cannot be removed in maintenance release)

throw new \RuntimeException('A Controller class name must end with Controller.');
}
} else {
throw new \LogicException('Unable to parse the Controller name.');
}
} else {
list($class, $method) = explode('::', $controller, 2);

if (!preg_match('/Controller$/', $class)) {
throw new \RuntimeException('A Controller class name must end with Controller.');
}
}

$request = $this->container->get('request');

if (!$this->container->get('routing.url_signer')->verify($request->getUri())) {
throw new \RuntimeException('Invalid url signature.');
}

$attributes = $request->attributes;

$attributes->remove('path');
$attributes->remove('controller');
if ('none' !== $path) {
parse_str($path, $tmp);
$attributes->add($tmp);
}

return $this->container->get('http_kernel')->forward($controller, $attributes->all(), $request->query->all());
}
}
101 changes: 68 additions & 33 deletions src/Symfony/Bundle/FrameworkBundle/HttpKernel.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,60 +84,75 @@ public function forward($controller, array $attributes = array(), array $query =
*
* Available options:
*
* * attributes: An array of request attributes (only when the first argument is a controller)
* * query: An array of request query parameters (only when the first argument is a controller)
* * ignore_errors: true to return an empty string in case of an error
* * alt: an alternative URI to execute in case of an error
* * alt: an alternative controller to execute in case of an error (can be a controller, a URI, or an array with the controller, the attributes, and the query arguments)
* * standalone: whether to generate an esi:include tag or not when ESI is supported
* * comment: a comment to add when returning an esi:include tag
*
* @param string $uri A URI
* @param array $options An array of options
* @param string $controller A controller name to execute (a string like BlogBundle:Post:index), or a relative URI
* @param array $options An array of options
*
* @return string The Response content
*
* @throws \RuntimeException
* @throws \Exception
*/
public function render($uri, array $options = array())
public function render($controller, array $options = array())
{
$request = $this->container->get('request');

$options = array_merge(array(
'attributes' => array(),
'query' => array(),
'ignore_errors' => !$this->container->getParameter('kernel.debug'),
'alt' => null,
'alt' => array(),
'standalone' => false,
'comment' => '',
'default' => null,
), $options);

if (!is_array($options['alt'])) {
$options['alt'] = array($options['alt']);
}

if (null === $this->esiSupport) {
$this->esiSupport = $this->container->has('esi') && $this->container->get('esi')->hasSurrogateEsiCapability($request);
$this->esiSupport = $this->container->has('esi') && $this->container->get('esi')->hasSurrogateEsiCapability($this->container->get('request'));
}

if ($this->esiSupport && (true === $options['standalone'] || 'esi' === $options['standalone'])) {
return $this->container->get('esi')->renderIncludeTag($uri, $options['alt'], $options['ignore_errors'], $options['comment']);
}
$uri = $this->generateInternalUri($controller, $options['attributes'], $options['query']);

if ('js' === $options['standalone']) {
$defaultContent = null;
$alt = '';
if ($options['alt']) {
$alt = $this->generateInternalUri($options['alt'][0], isset($options['alt'][1]) ? $options['alt'][1] : array(), isset($options['alt'][2]) ? $options['alt'][2] : array());
}

$templating = $this->container->get('templating');
return $this->container->get('esi')->renderIncludeTag($uri, $alt, $options['ignore_errors'], $options['comment']);
}

$request = $this->container->get('request');

if ($options['default']) {
if ($templating->exists($options['default'])) {
$defaultContent = $templating->render($options['default']);
} else {
$defaultContent = $options['default'];
}
} elseif ($template = $this->container->getParameter('templating.hinclude.default_template')) {
$defaultContent = $templating->render($template);
// controller or URI or path?
if (0 === strpos($controller, 'http://') || 0 === strpos($controller, 'https://')) {
$subRequest = Request::create($controller, 'get', array(), $request->cookies->all(), array(), $request->server->all());
if ($session = $request->getSession()) {
$subRequest->setSession($session);
}
} elseif (0 === strpos($controller, '/')) {
$subRequest = Request::create($request->getUriForPath($controller), 'get', array(), $request->cookies->all(), array(), $request->server->all());
if ($session = $request->getSession()) {
$subRequest->setSession($session);
}
} else {
$options['attributes']['_controller'] = $controller;

return $this->renderHIncludeTag($uri, $defaultContent);
}
if (!isset($options['attributes']['_format'])) {
$options['attributes']['_format'] = $request->getRequestFormat();
}

$subRequest = Request::create($uri, 'get', array(), $request->cookies->all(), array(), $request->server->all());
if ($session = $request->getSession()) {
$subRequest->setSession($session);
$options['attributes']['_route'] = '_internal';
$subRequest = $request->duplicate($options['query'], null, $options['attributes']);
$subRequest->setMethod('GET');
}

$level = ob_get_level();
Expand All @@ -157,8 +172,10 @@ public function render($uri, array $options = array())
if ($options['alt']) {
$alt = $options['alt'];
unset($options['alt']);
$options['attributes'] = isset($alt[1]) ? $alt[1] : array();
$options['query'] = isset($alt[2]) ? $alt[2] : array();

return $this->render($alt, $options);
return $this->render($alt[0], $options);
}

if (!$options['ignore_errors']) {
Expand All @@ -173,16 +190,34 @@ public function render($uri, array $options = array())
}

/**
* Renders an HInclude tag.
* Generates an internal URI for a given controller.
*
* @param string $uri A URI
* @param string $defaultContent Default content
* This method uses the "_internal" route, which should be available.
*
* @return string
* @param string $controller A controller name to execute (a string like BlogBundle:Post:index), or a relative URI
* @param array $attributes An array of request attributes
* @param array $query An array of request query parameters
*
* @return string An internal URI
*/
public function renderHIncludeTag($uri, $defaultContent = null)
public function generateInternalUri($controller, array $attributes = array(), array $query = array())
{
return sprintf('<hx:include src="%s">%s</hx:include>', $uri, $defaultContent);
if (0 === strpos($controller, '/')) {
return $controller;
}

$path = http_build_query($attributes, '', '&');
$uri = $this->container->get('router')->generate('_internal', array(
'controller' => $controller,
'path' => $path ?: 'none',
'_format' => $this->container->get('request')->getRequestFormat(),
));

if ($queryString = http_build_query($query, '', '&')) {
$uri .= '?'.$queryString;
}

return $this->container->get('routing.url_signer')->sign($uri);
}

public function hasEsiSupport()
Expand Down
5 changes: 5 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/esi.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<parameters>
<parameter key="esi.class">Symfony\Component\HttpKernel\HttpCache\Esi</parameter>
<parameter key="esi_listener.class">Symfony\Component\HttpKernel\EventListener\EsiListener</parameter>
<parameter key="routing.url_signer.class">Symfony\Bundle\FrameworkBundle\Routing\UrlSigner</parameter>
</parameters>

<services>
Expand All @@ -16,5 +17,9 @@
<tag name="kernel.event_subscriber" />
<argument type="service" id="esi" on-invalid="ignore" />
</service>

<service id="routing.url_signer" class="%routing.url_signer.class%">
<argument>%kernel.secret%</argument>
</service>
</services>
</container>
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?xml version="1.0" encoding="UTF-8" ?>

<routes xmlns="http://symfony.com/schema/routing"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/routing http://symfony.com/schema/routing/routing-1.0.xsd">

<route id="_internal" pattern="/secure/{controller}/{path}.{_format}">
<default key="_controller">FrameworkBundle:Internal:index</default>
<requirement key="path">.+</requirement>
<requirement key="_format">[^.]+</requirement>
</route>

<route id="_internal_public" pattern="/{controller}/{path}.{_format}">
<default key="_controller">FrameworkBundle:Internal:index</default>
<requirement key="path">.+</requirement>
<requirement key="_format">[^.]+</requirement>
</route>
</routes>
52 changes: 52 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Routing/UrlSigner.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\FrameworkBundle\Routing;

/**
* Signs and verifies signed urls.
*
* @author Alexander <iam.asm89@gmail.com>
*/
class UrlSigner
{
private $secret;

/**
* @param string $secret
*/
public function __construct($secret)
{
$this->secret = $secret;
}

/**
* @param string $url
*
* @return string Signed url.
*/
public function sign($url)
{
// todo: actually sign the url
return $url;
}

/**
* @param string $url
*
* @return boolean
*/
public function verify($url)
{
// todo: actually verify the signed url
return true;
}
}
Loading