Skip to content

Commit

Permalink
Merge pull request PrestaShop#35761 from M0rgan01/fix/35487
Browse files Browse the repository at this point in the history
Introduce PS_TRUSTED_PROXIES env, and delete the obsolete PS_SSL_ENABLED_EVERYWHERE config
  • Loading branch information
M0rgan01 authored Apr 2, 2024
2 parents 618020b + bbdee4b commit a1df645
Show file tree
Hide file tree
Showing 15 changed files with 25 additions and 49 deletions.
1 change: 1 addition & 0 deletions .env
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@

# Enable new Symfony based front container
PS_FF_FRONT_CONTAINER_V2=false
PS_TRUSTED_PROXIES=
1 change: 1 addition & 0 deletions .env.dist
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
PS_CONTAINER_V2_FRONT=false
PS_TRUSTED_PROXIES=
2 changes: 1 addition & 1 deletion admin-dev/filemanager/config/config.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
// | | | | |- plugin.min.js

$base_url = Tools::getHttpHost(true); // DON'T TOUCH (base url (only domain) of site (without final /)).
$base_url = Configuration::get('PS_SSL_ENABLED') && Configuration::get('PS_SSL_ENABLED_EVERYWHERE') ? $base_url : str_replace('https', 'http', $base_url);
$base_url = Configuration::get('PS_SSL_ENABLED') ? $base_url : str_replace('https', 'http', $base_url);
$upload_dir = Context::getContext()->shop->getBaseURI().'img/cms/'; // path from base_url to base of upload folder (with start and final /)
$current_path = _PS_ROOT_DIR_.'/img/cms/'; // relative path from filemanager folder to upload folder (with final /)
//thumbs folder can't put inside upload folder
Expand Down
4 changes: 4 additions & 0 deletions app/config/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ parameters:
admin_page: "%prestashop_views%/Admin"
env(PS_LOG_OUTPUT): "%kernel.logs_dir%/%kernel.environment%.log"
env(PS_LOG_MAX_FILES): '30'
env(PS_TRUSTED_PROXIES): ''
mail_themes_uri: "/mails/themes"
mail_themes_dir: "%kernel.project_dir%%mail_themes_uri%"
modules_translation_paths: [ ]
Expand All @@ -38,6 +39,9 @@ services:
public: true

framework:
# proxies configuration, see https://symfony.com/doc/current/deployment/proxies.html
trusted_proxies: '%env(PS_TRUSTED_PROXIES)%'

assets:
version: !php/const PrestaShop\PrestaShop\Core\Version::VERSION
packages:
Expand Down
4 changes: 2 additions & 2 deletions classes/Link.php
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,7 @@ public function getLegacyAdminLink($controller, $withToken = true, $params = [])
public function getAdminBaseLink($idShop = null, $ssl = null, $relativeProtocol = false)
{
if (null === $ssl) {
$ssl = Configuration::get('PS_SSL_ENABLED') && Configuration::get('PS_SSL_ENABLED_EVERYWHERE');
$ssl = Configuration::get('PS_SSL_ENABLED');
}

if (Configuration::get('PS_MULTISHOP_FEATURE_ACTIVE')) {
Expand Down Expand Up @@ -1353,7 +1353,7 @@ protected function getLangLink($idLang = null, Context $context = null, $idShop
public function getBaseLink($idShop = null, $ssl = null, $relativeProtocol = false)
{
if (null === $ssl) {
$ssl = (Configuration::get('PS_SSL_ENABLED') && Configuration::get('PS_SSL_ENABLED_EVERYWHERE'));
$ssl = Configuration::get('PS_SSL_ENABLED');
}

if (Configuration::get('PS_MULTISHOP_FEATURE_ACTIVE') && $idShop !== null) {
Expand Down
2 changes: 1 addition & 1 deletion classes/controller/FrontController.php
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public function __construct()

parent::__construct();

if (Configuration::get('PS_SSL_ENABLED') && Configuration::get('PS_SSL_ENABLED_EVERYWHERE')) {
if (Configuration::get('PS_SSL_ENABLED')) {
$this->ssl = true;
}

Expand Down
2 changes: 1 addition & 1 deletion config/config.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@
$cookie_lifetime = time() + (max($cookie_lifetime, 1) * 3600);
}

$force_ssl = Configuration::get('PS_SSL_ENABLED') && Configuration::get('PS_SSL_ENABLED_EVERYWHERE');
$force_ssl = Configuration::get('PS_SSL_ENABLED');
if (defined('_PS_ADMIN_DIR_')) {
$cookie = new Cookie('psAdmin', '', $cookie_lifetime, null, false, $force_ssl);
} else {
Expand Down
3 changes: 1 addition & 2 deletions src/Adapter/GeneralConfiguration.php
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,8 @@ public function validateConfiguration(array $configuration)
*/
protected function validateSameSite(string $sameSite): bool
{
$forceSsl = $this->configuration->get('PS_SSL_ENABLED') && $this->configuration->get('PS_SSL_ENABLED_EVERYWHERE');
if ($sameSite === CookieOptions::SAMESITE_NONE) {
return $forceSsl;
return $this->configuration->get('PS_SSL_ENABLED');
}

return true;
Expand Down
11 changes: 1 addition & 10 deletions src/Adapter/Preferences/PreferencesConfiguration.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ public function getConfiguration()
{
return [
'enable_ssl' => $this->configuration->getBoolean('PS_SSL_ENABLED'),
'enable_ssl_everywhere' => $this->configuration->getBoolean('PS_SSL_ENABLED_EVERYWHERE'),
'enable_token' => $this->configuration->getBoolean('PS_TOKEN_ENABLE'),
'allow_html_iframes' => $this->configuration->getBoolean('PS_ALLOW_HTML_IFRAME'),
'use_htmlpurifier' => $this->configuration->getBoolean('PS_USE_HTMLPURIFIER'),
Expand Down Expand Up @@ -91,10 +90,7 @@ public function updateConfiguration(array $configuration)
];
}

$previousMultistoreFeatureState = $this->configuration->get('PS_MULTISHOP_FEATURE_ACTIVE');

$this->configuration->set('PS_SSL_ENABLED', $configuration['enable_ssl']);
$this->configuration->set('PS_SSL_ENABLED_EVERYWHERE', $configuration['enable_ssl_everywhere']);
$this->configuration->set('PS_TOKEN_ENABLE', $configuration['enable_token']);
$this->configuration->set('PS_ALLOW_HTML_IFRAME', $configuration['allow_html_iframes']);
$this->configuration->set('PS_USE_HTMLPURIFIER', $configuration['use_htmlpurifier']);
Expand All @@ -118,11 +114,7 @@ public function updateConfiguration(array $configuration)
*/
protected function validateSameSiteConfiguration(array $configuration): bool
{
return (
$configuration['enable_ssl'] === false
|| $configuration['enable_ssl_everywhere'] === false
)
&& $this->configuration->get('PS_COOKIE_SAMESITE') === CookieOptions::SAMESITE_NONE;
return $configuration['enable_ssl'] === false && $this->configuration->get('PS_COOKIE_SAMESITE') === CookieOptions::SAMESITE_NONE;
}

/**
Expand All @@ -132,7 +124,6 @@ public function validateConfiguration(array $configuration)
{
return isset(
$configuration['enable_ssl'],
$configuration['enable_ssl_everywhere'],
$configuration['enable_token'],
$configuration['allow_html_iframes'],
$configuration['use_htmlpurifier'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@
namespace PrestaShopBundle\EventListener\Admin;

use PrestaShop\PrestaShop\Core\ConfigurationInterface;
use RuntimeException;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpKernel\Event\RequestEvent;

/**
* Middleware that is triggered during `kernel.request` event on Symfony routing process, to redirect to HTTPS in some cases.
*
* If PS_SSL_ENABLED & (PS_SSL_ENABLED_EVERYWHERE | REFERER is HTTPS)
* If PS_SSL_ENABLED & REFERER is HTTPS
* Then redirect to the equivalent URL to HTTPS.
*/
class SSLMiddlewareListener
Expand All @@ -61,11 +62,16 @@ public function onKernelRequest(RequestEvent $event): void

//If It's Sf route and SSL enabled and forced, redirect to https
$enabled = (1 === (int) $this->configuration->get('PS_SSL_ENABLED'));
$forced = (1 === (int) $this->configuration->get('PS_SSL_ENABLED_EVERYWHERE'));
$serverParams = $event->getRequest()->server;
$refererSsl = ($serverParams->has('HTTP_REFERER') && str_starts_with($serverParams->get('HTTP_REFERER'), 'https'));

if ($enabled && ($forced || $refererSsl)) {
if ($enabled && $refererSsl) {
$forwardedProto = ($serverParams->has('HTTP_X_FORWARDED_PROTO') && str_starts_with($serverParams->get('HTTP_X_FORWARDED_PROTO'), 'https'));

if ($forwardedProto) {
throw new RuntimeException("Infinite redirection detected, please fill in the 'PS_TRUSTED_PROXIES' environment variable or disable the PS_SSL_ENABLED configuration");
}

$this->redirectToSsl($event);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,12 @@ final class GeneralDataProvider implements FormDataProviderInterface
*/
private $sslEnabled;

/**
* @var bool
*/
private $sslEnabledEverywhere;

public function __construct(
DataConfigurationInterface $dataConfiguration,
bool $sslEnabled,
bool $sslEnabledEverywhere
bool $sslEnabled
) {
$this->dataConfiguration = $dataConfiguration;
$this->sslEnabled = $sslEnabled;
$this->sslEnabledEverywhere = $sslEnabledEverywhere;
}

/**
Expand Down Expand Up @@ -146,7 +139,7 @@ private function validate(array $data): void
protected function validateSameSite(string $sameSite): bool
{
if ($sameSite === CookieOptions::SAMESITE_NONE) {
return $this->sslEnabled && $this->sslEnabledEverywhere;
return $this->sslEnabled;
}

return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,30 +98,18 @@ public function __construct(
public function buildForm(FormBuilderInterface $builder, array $options)
{
$configuration = $this->configuration;
$isSslEnabled = (bool) $configuration->get('PS_SSL_ENABLED');

if ($this->requestStack->getCurrentRequest()->isSecure()) {
$builder->add('enable_ssl', SwitchType::class, [
'label' => $this->trans('Enable SSL', 'Admin.Shopparameters.Feature'),
'help' => $this->trans(
'If you own an SSL certificate for your shop\'s domain name, you can activate SSL encryption (https://) for customer account identification and order processing.',
'If you own an SSL certificate for your shop\'s domain name, you can activate SSL encryption (https://) for all the pages of your shop.',
'Admin.Shopparameters.Help'
),
]);
}

$builder
->add('enable_ssl_everywhere', SwitchType::class, [
'disabled' => !$isSslEnabled,
'label' => $this->trans(
'Enable SSL on all pages',
'Admin.Shopparameters.Feature'
),
'help' => $this->trans(
'When enabled, all the pages of your shop will be SSL-secured.',
'Admin.Shopparameters.Help'
),
])
->add('enable_token', SwitchType::class, [
'disabled' => !$this->isContextDependantOptionEnabled(),
'label' => $this->trans(
Expand Down
1 change: 0 additions & 1 deletion src/PrestaShopBundle/Install/Install.php
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,6 @@ public function configureShop(array $data = [])

// Set SSL configuration
Configuration::updateGlobalValue('PS_SSL_ENABLED', (int) $data['enable_ssl']);
Configuration::updateGlobalValue('PS_SSL_ENABLED_EVERYWHERE', (int) $data['enable_ssl']);

// Set mails configuration
Configuration::updateGlobalValue('PS_MAIL_METHOD', ($data['use_smtp']) ? 2 : 1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ services:
arguments:
- '@prestashop.adapter.general.configuration'
- '@=service("prestashop.adapter.legacy.configuration").getBoolean("PS_SSL_ENABLED")'
- '@=service("prestashop.adapter.legacy.configuration").getBoolean("PS_SSL_ENABLED_EVERYWHERE")'

prestashop.adapter.administration.upload_quota.form_provider:
class: 'PrestaShopBundle\Form\Admin\Configure\AdvancedParameters\Administration\UploadQuotaDataProvider'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ public function testGetConfiguration()
->willReturnMap(
[
['PS_SSL_ENABLED', false, true],
['PS_SSL_ENABLED_EVERYWHERE', false, true],
['PS_TOKEN_ENABLE', false, true],
['PS_ALLOW_HTML_IFRAME', false, true],
['PS_USE_HTMLPURIFIER', false, true],
Expand All @@ -86,7 +85,6 @@ public function testGetConfiguration()
$this->assertSame(
[
'enable_ssl' => true,
'enable_ssl_everywhere' => true,
'enable_token' => true,
'allow_html_iframes' => true,
'use_htmlpurifier' => true,
Expand Down Expand Up @@ -137,7 +135,6 @@ public function testUpdateConfigurationWithInvalidSSLConfiguration()
$this->object->updateConfiguration(
[
'enable_ssl' => false,
'enable_ssl_everywhere' => false,
'enable_token' => true,
'allow_html_iframes' => true,
'use_htmlpurifier' => true,
Expand Down Expand Up @@ -166,7 +163,6 @@ public function testUpdateConfiguration()
->willReturnMap(
[
['PS_SSL_ENABLED', true],
['PS_SSL_ENABLED_EVERYWHERE', true],
['PS_TOKEN_ENABLE', true],
['PS_ALLOW_HTML_IFRAME', true],
['PS_USE_HTMLPURIFIER', true],
Expand All @@ -191,7 +187,6 @@ public function testUpdateConfiguration()
$this->object->updateConfiguration(
[
'enable_ssl' => false,
'enable_ssl_everywhere' => false,
'enable_token' => true,
'allow_html_iframes' => true,
'use_htmlpurifier' => true,
Expand Down

0 comments on commit a1df645

Please sign in to comment.