Skip to content

Commit 0b03afc

Browse files
mitelgmitelg
committed
Merge branch 'sw-26689/set-cookie-secure' into '5.7'
SW-26689 - Set secure attribute depending on the shop configuration See merge request shopware/5/product/shopware!803
2 parents 8d20e10 + 0da9be0 commit 0b03afc

File tree

3 files changed

+107
-27
lines changed

3 files changed

+107
-27
lines changed

‎engine/Shopware/Components/CSRFTokenValidator.php

Lines changed: 47 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,8 @@ public function checkFrontendTokenValidation(Enlight_Event_EventArgs $args)
148148
}
149149

150150
if ($request->isGet() && !$this->isProtected($controller)) {
151+
$this->updateCsrfTokenIfNotAvailable($request, $response);
152+
151153
return;
152154
}
153155

@@ -171,32 +173,63 @@ public function checkFrontendTokenValidation(Enlight_Event_EventArgs $args)
171173

172174
public function regenerateToken(Request $request, Response $response): string
173175
{
174-
$context = $this->contextService->getShopContext();
175-
176-
$name = self::CSRF_SESSION_KEY . $context->getShop()->getId();
177-
178-
if ($context->getShop()->getParentId() && $this->componentsConfig->get('shareSessionBetweenLanguageShops')) {
179-
$name = self::CSRF_SESSION_KEY . $context->getShop()->getParentId();
180-
}
176+
$shop = $this->contextService->getShopContext()->getShop();
177+
$name = $this->getCsrfName();
181178

182179
$token = Random::getAlphanumericString(30);
183180
$this->container->get('session')->set($name, $token);
184-
$response->headers->setCookie(new Cookie($name, $token, 0, '/', null, $request->isSecure(), false));
181+
182+
$response->headers->clearCookie($name);
183+
184+
/*
185+
* Appending a '/' to the $basePath is not strictly necessary, but it is
186+
* done to all cookie base paths in the
187+
* `themes/Frontend/Bare/frontend/index/index.tpl` template. It's done
188+
* here as well for compatibility reasons.
189+
*/
190+
$response->headers->setCookie(new Cookie(
191+
$name,
192+
$token,
193+
0,
194+
sprintf('%s/', $shop->getPath() ?: ''),
195+
null,
196+
$shop->getSecure(),
197+
false
198+
));
185199

186200
return $token;
187201
}
188202

203+
private function updateCsrfTokenIfNotAvailable(Request $request, Response $response): void
204+
{
205+
$name = $this->getCsrfName();
206+
207+
if ($this->container->get('session')->has($name)) {
208+
return;
209+
}
210+
211+
$this->regenerateToken($request, $response);
212+
}
213+
214+
private function getCsrfName(): string
215+
{
216+
$shop = $this->contextService->getShopContext()->getShop();
217+
218+
$name = self::CSRF_SESSION_KEY . $shop->getId();
219+
220+
if ($shop->getParentId() && $this->componentsConfig->get('shareSessionBetweenLanguageShops')) {
221+
$name = self::CSRF_SESSION_KEY . $shop->getParentId();
222+
}
223+
224+
return $name;
225+
}
226+
189227
/**
190228
* Check if the submitted CSRF token in cookie or header matches with the token stored in the session
191229
*/
192230
private function checkRequest(Request $request, Response $response): bool
193231
{
194-
$context = $this->contextService->getShopContext();
195-
$name = self::CSRF_SESSION_KEY . $context->getShop()->getId();
196-
197-
if ($context->getShop()->getParentId() && $this->componentsConfig->get('shareSessionBetweenLanguageShops')) {
198-
$name = self::CSRF_SESSION_KEY . $context->getShop()->getParentId();
199-
}
232+
$name = $this->getCsrfName();
200233

201234
$token = $this->container->get('session')->get($name);
202235

‎tests/Functional/Components/CSRFTokenValidatorTest.php

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,14 @@ class CSRFTokenValidatorTest extends TestCase
4747

4848
public const EXISTING_ACTION_NAME = 'foo';
4949

50+
private const CSRF_TOKEN_FOR_SHOP_ONE = '__csrf_token-1';
51+
5052
/**
5153
* @before
5254
*/
5355
public function enableCsrfInFrontend(): void
5456
{
57+
$this->getContainer()->get('session')->offsetUnset(self::CSRF_TOKEN_FOR_SHOP_ONE);
5558
Utils::hijackProperty($this->getContainer()->get(CSRFTokenValidator::class), 'isEnabledFrontend', true);
5659
}
5760

@@ -60,6 +63,7 @@ public function enableCsrfInFrontend(): void
6063
*/
6164
public function disableCsrfInFrontend(): void
6265
{
66+
$this->getContainer()->get('session')->offsetUnset(self::CSRF_TOKEN_FOR_SHOP_ONE);
6367
Utils::hijackProperty($this->getContainer()->get(CSRFTokenValidator::class), 'isEnabledFrontend', false);
6468
}
6569

@@ -85,7 +89,7 @@ public function testFrontendTokenIsValid(): void
8589

8690
$tokenValidator->checkFrontendTokenValidation($enlightEventArgs);
8791

88-
static::assertNotNull($this->getContainer()->get('session')->get('__csrf_token-1'));
92+
static::assertIsString($this->getContainer()->get('session')->get(self::CSRF_TOKEN_FOR_SHOP_ONE));
8993
static::assertTrue($incomingRequest->getAttribute('isValidated'));
9094
}
9195

@@ -115,8 +119,8 @@ public function testFrontendTokenValidationThrowsError(): void
115119
static::assertInstanceOf(CSRFTokenValidationException::class, $e);
116120
}
117121

118-
static::assertNotNull($this->getContainer()->get('session')->get('__csrf_token-1'));
119-
static::assertNotEquals($token, $this->getContainer()->get('session')->get('__csrf_token-1'));
122+
static::assertIsString($this->getContainer()->get('session')->get(self::CSRF_TOKEN_FOR_SHOP_ONE));
123+
static::assertNotEquals($token, $this->getContainer()->get('session')->get(self::CSRF_TOKEN_FOR_SHOP_ONE));
120124
}
121125

122126
public function testCsrfExceptionIsThrownWhenNoSession(): void
@@ -141,7 +145,7 @@ public function testCsrfExceptionIsThrownWhenNoSession(): void
141145
static::assertInstanceOf(CSRFTokenValidationException::class, $e);
142146
}
143147

144-
static::assertNotNull($this->getContainer()->get('session')->get('__csrf_token-1'));
148+
static::assertIsString($this->getContainer()->get('session')->get(self::CSRF_TOKEN_FOR_SHOP_ONE));
145149
}
146150

147151
public function testCsrfExceptionIsThrownWhenNoRequestCsrfIsSet(): void
@@ -167,7 +171,51 @@ public function testCsrfExceptionIsThrownWhenNoRequestCsrfIsSet(): void
167171
static::assertInstanceOf(CSRFTokenValidationException::class, $e);
168172
}
169173

170-
static::assertNotNull($this->getContainer()->get('session')->get('__csrf_token-1'));
174+
static::assertIsString($this->getContainer()->get('session')->get(self::CSRF_TOKEN_FOR_SHOP_ONE));
175+
}
176+
177+
public function testCsrfTokenIsUpdatedIfItIsNotAvailableInTheSessionAndIsGetRequest(): void
178+
{
179+
$tokenValidator = $this->getContainer()->get(CSRFTokenValidator::class);
180+
$this->getContainer()->get(ContextServiceInterface::class)->createShopContext(1);
181+
182+
static::assertNull($this->getContainer()->get('session')->get(self::CSRF_TOKEN_FOR_SHOP_ONE));
183+
184+
$controller = new NotProtectionAwareController();
185+
$incomingRequest = new Enlight_Controller_Request_RequestTestCase();
186+
$incomingRequest->setMethod('GET');
187+
$createResponse = new Enlight_Controller_Response_ResponseTestCase();
188+
$controller->setRequest($incomingRequest);
189+
$controller->setResponse($createResponse);
190+
$enlightEventArgs = new Enlight_Event_EventArgs([
191+
'subject' => $controller,
192+
]);
193+
194+
$tokenValidator->checkFrontendTokenValidation($enlightEventArgs);
195+
196+
static::assertIsString($this->getContainer()->get('session')->get(self::CSRF_TOKEN_FOR_SHOP_ONE));
197+
}
198+
199+
public function testCsrfTokenIsNotUpdatedIfItIsNotAvailableInTheSession(): void
200+
{
201+
$tokenValidator = $this->getContainer()->get(CSRFTokenValidator::class);
202+
$this->getContainer()->get(ContextServiceInterface::class)->createShopContext(1);
203+
204+
static::assertNull($this->getContainer()->get('session')->get(self::CSRF_TOKEN_FOR_SHOP_ONE));
205+
206+
$controller = new MockController();
207+
$incomingRequest = new Enlight_Controller_Request_RequestTestCase();
208+
$incomingRequest->setMethod('GET');
209+
$incomingRequest->setActionName(self::EXISTING_ACTION_NAME);
210+
$createResponse = new Enlight_Controller_Response_ResponseTestCase();
211+
$controller->setRequest($incomingRequest);
212+
$controller->setResponse($createResponse);
213+
$enlightEventArgs = new Enlight_Event_EventArgs([
214+
'subject' => $controller,
215+
]);
216+
217+
$this->expectException(CSRFTokenValidationException::class);
218+
$tokenValidator->checkFrontendTokenValidation($enlightEventArgs);
171219
}
172220
}
173221

@@ -180,3 +228,7 @@ public function getCSRFProtectedActions()
180228
];
181229
}
182230
}
231+
232+
class NotProtectionAwareController extends Enlight_Controller_Action
233+
{
234+
}

‎themes/Frontend/Responsive/frontend/_public/src/js/jquery.csrf-protection.js

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,6 @@
174174
$.ajax({
175175
url: window.csrfConfig.generateUrl,
176176
success: function(response, status, xhr) {
177-
me.saveToken(xhr.getResponseHeader('x-csrf-token'));
178177
$.publish('plugin/swCsrfProtection/requestToken', [me, me.getToken()]);
179178
me.afterInit();
180179
}
@@ -183,15 +182,11 @@
183182
},
184183

185184
/**
186-
* Save token into a cookie
185+
* @deprecated will be removed with v5.8.0
186+
*
187187
* @param token
188188
*/
189-
saveToken: function(token) {
190-
var me = this,
191-
basePath = window.csrfConfig.basePath || '/';
192-
193-
document.cookie = me.storageKey + '=' + token + '; path=' + basePath + ($.isSecure() ? '; secure;' : '');
194-
},
189+
saveToken: function(token) {},
195190

196191
/**
197192
* Initialize the CSRF protection

0 commit comments

Comments
 (0)