Skip to content

Assert AttributeGroup create Location header has no admin token - #285

Open
boo-code wants to merge 2 commits into
PrestaShop:devfrom
boo-code:fix/api-location-no-token
Open

boo-code wants to merge 2 commits into
PrestaShop:devfrom
boo-code:fix/api-location-no-token

Conversation

@boo-code

@boo-code boo-code commented Jun 26, 2026 •

Copy link
Copy Markdown
Questions Answers
Branch? dev
Description? Assert the AttributeGroup create response Location header carries no admin _token.
Type? bug fix
Fixed issue Companion to PrestaShop/PrestaShop#41847 (fixes PrestaShop/PrestaShop#39496)

Context

PrestaShop/PrestaShop#39496: Admin API IRIs leaked the admin CSRF _token. The core fix PrestaShop/PrestaShop#41847 stops adding it to API Platform routes.

This adds a regression test asserting the POST /attributes/groups Location header has no _token.

Verification

Against an install with the core fix: green. Without it: red (Location: /attributes/groups/5?_token=…).

The test starts with markTestSkippedByMinVersion('9.2.1'), the milestone of PrestaShop/PrestaShop#42221. The 9.2.x branch already reports 9.2.1, so the 9.2.x and develop legs run it and stay red until PrestaShop/PrestaShop#42221 merges.

@ps-jarvis

Copy link
Copy Markdown

Hello @boo-code!

This is your first pull request on ps_apiresources repository of the PrestaShop project.

Thank you, and welcome to this Open Source community!

* The Admin API is stateless and OAuth-authenticated, so the IRIs it returns (here the
* Location header of a POST) must not carry the admin CSRF _token. See PrestaShop/PrestaShop#39496.
*/
public function testCreateResponseLocationHeaderHasNoAdminToken(): void

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No core carries the fix yet, and 9.0.3 / 9.1.5 in the CI matrix never will, so this test needs a version gate. Could you start it with $this->markTestSkippedByMinVersion('9.2.1'), or whichever 9.2 patch ends up shipping PrestaShop/PrestaShop#42221?

],
]);
self::assertResponseStatusCodeSame(201);
$location = $response->getHeaders(false)['location'][0] ?? '';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

With ?? '', a response without any Location header passes too. Asserting the IRI itself, e.g. assertMatchesRegularExpression('#/attributes/groups/\d+$#', $location), would cover both cases.

@ps-jarvis ps-jarvis moved this from Ready for review to Waiting for author in PR Dashboard Oct 29, 2026
@boo-code
boo-code force-pushed the fix/api-location-no-token branch from 1d00184 to c1ddcc0 Compare October 29, 2026 18:47
@boo-code

Copy link
Copy Markdown
Author

Done: the test starts with markTestSkippedByMinVersion('9.2.1'), the milestone of PrestaShop/PrestaShop#42221, and asserts #/attributes/groups/\d+$#. Note 9.2.x already reports 9.2.1, so the 9.2.x and develop legs run it and stay red until PrestaShop/PrestaShop#42221 merges. On a 9.2.1 core: red without the core change (/attributes/groups/5?_token=...), green with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Waiting for author

Development

Successfully merging this pull request may close these issues.

Admin API location header includes useless _token

3 participants