Skip to content

Commit e43e9c3

Browse files
authored
Merge pull request #2811 from weaverryan/backport-original-data
Backport original_data access control variable to 2.4 as a bug fix
2 parents cb4f3cd + 54ca5ef commit e43e9c3

File tree

6 files changed

+36
-6
lines changed

6 files changed

+36
-6
lines changed

features/authorization/deny.feature

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,39 @@ Feature: Authorization checking
5858
"""
5959
Then the response status code should be 201
6060

61-
Scenario: An user cannot retrieve an item he doesn't own
61+
Scenario: A user cannot retrieve an item he doesn't own
6262
When I add "Accept" header equal to "application/ld+json"
6363
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
6464
And I send a "GET" request to "/secured_dummies/1"
6565
Then the response status code should be 403
6666
And the response should be in JSON
6767

68-
Scenario: An user can retrieve an item he owns
68+
Scenario: A user can retrieve an item he owns
6969
When I add "Accept" header equal to "application/ld+json"
7070
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
7171
And I send a "GET" request to "/secured_dummies/2"
7272
Then the response status code should be 200
73+
74+
Scenario: A user can't assign him an item he doesn't own
75+
When I add "Accept" header equal to "application/ld+json"
76+
And I add "Content-Type" header equal to "application/ld+json"
77+
And I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
78+
And I send a "PUT" request to "/secured_dummies/2" with body:
79+
"""
80+
{
81+
"owner": "kitten"
82+
}
83+
"""
84+
Then the response status code should be 403
85+
86+
Scenario: A user can update an item he owns and transfer it
87+
When I add "Accept" header equal to "application/ld+json"
88+
And I add "Content-Type" header equal to "application/ld+json"
89+
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
90+
And I send a "PUT" request to "/secured_dummies/2" with body:
91+
"""
92+
{
93+
"owner": "vincent"
94+
}
95+
"""
96+
Then the response status code should be 200

src/EventListener/ReadListener.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,5 +115,6 @@ public function onKernelRequest(GetResponseEvent $event): void
115115
}
116116

117117
$request->attributes->set('data', $data);
118+
$request->attributes->set('previous_data', \is_object($data) ? clone $data : $data);
118119
}
119120
}

src/Security/EventListener/DenyAccessListener.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,6 @@ public function __construct(ResourceMetadataFactoryInterface $resourceMetadataFa
5050
}
5151

5252
/**
53-
* Sets the applicable format to the HttpFoundation Request.
54-
*
5553
* @throws AccessDeniedException
5654
*/
5755
public function onKernelRequest(GetResponseEvent $event): void
@@ -71,6 +69,7 @@ public function onKernelRequest(GetResponseEvent $event): void
7169

7270
$extraVariables = $request->attributes->all();
7371
$extraVariables['object'] = $request->attributes->get('data');
72+
$extraVariables['previous_object'] = $request->attributes->get('previous_data');
7473
$extraVariables['request'] = $request;
7574

7675
if (!$this->resourceAccessChecker->isGranted($attributes['resource_class'], $isGranted, $extraVariables)) {

tests/EventListener/ReadListenerTest.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ public function testRetrieveCollectionPost()
153153
$listener->onKernelRequest($event->reveal());
154154

155155
$this->assertFalse($request->attributes->has('data'));
156+
$this->assertFalse($request->attributes->has('previous_data'));
156157
}
157158

158159
public function testRetrieveCollectionGet()
@@ -178,6 +179,7 @@ public function testRetrieveCollectionGet()
178179
$listener->onKernelRequest($event->reveal());
179180

180181
$this->assertSame([], $request->attributes->get('data'));
182+
$this->assertFalse($request->attributes->has('previous_data'));
181183
}
182184

183185
public function testRetrieveItem()
@@ -205,6 +207,7 @@ public function testRetrieveItem()
205207
$listener->onKernelRequest($event->reveal());
206208

207209
$this->assertSame($data, $request->attributes->get('data'));
210+
$this->assertEquals($data, $request->attributes->get('previous_data'));
208211
}
209212

210213
public function testRetrieveItemNoIdentifier()
@@ -257,6 +260,7 @@ public function testRetrieveSubresource()
257260
$listener->onKernelRequest($event->reveal());
258261

259262
$this->assertSame($data, $request->attributes->get('data'));
263+
$this->assertSame($data, $request->attributes->get('previous_data'));
260264
}
261265

262266
public function testRetrieveSubresourceNoDataProvider()

tests/Fixtures/TestBundle/Document/SecuredDummy.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@
3030
* "post"={"access_control"="has_role('ROLE_ADMIN')"}
3131
* },
3232
* itemOperations={
33-
* "get"={"access_control"="has_role('ROLE_USER') and object.getOwner() == user"}
33+
* "get"={"access_control"="has_role('ROLE_USER') and object.getOwner() == user"},
34+
* "put"={"access_control"="has_role('ROLE_USER') and previous_object.getOwner() == user"},
3435
* },
3536
* graphql={
3637
* "query"={},

tests/Fixtures/TestBundle/Entity/SecuredDummy.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@
2929
* "post"={"access_control"="has_role('ROLE_ADMIN')"}
3030
* },
3131
* itemOperations={
32-
* "get"={"access_control"="has_role('ROLE_USER') and object.getOwner() == user"}
32+
* "get"={"access_control"="has_role('ROLE_USER') and object.getOwner() == user"},
33+
* "put"={"access_control"="has_role('ROLE_USER') and previous_object.getOwner() == user"},
3334
* },
3435
* graphql={
3536
* "query"={},

0 commit comments

Comments
 (0)