Skip to content

Commit 81eb8d9

Browse files
kbondkbond
committed
feature #1703 [make:user] Hash passwords using crc32c and deprecate eraseCredentials() (nicolas-grekas)
This PR was merged into the 1.x branch. Discussion ---------- [make:user] Hash passwords using crc32c and deprecate eraseCredentials() Fix #1700 Replace ##1701 Commits ------- 75886fb [make:user] Hash passwords using crc32c and deprecate eraseCredentials()
2 parents 8101ae3 + 75886fb commit 81eb8d9

11 files changed

+165
-27
lines changed

‎composer.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
"symfony/http-client": "^6.4|^7.0",
3535
"symfony/phpunit-bridge": "^6.4.1|^7.0",
3636
"symfony/security-core": "^6.4|^7.0",
37+
"symfony/security-http": "^6.4|^7.0",
3738
"symfony/yaml": "^6.4|^7.0",
3839
"twig/twig": "^3.0|^4.x-dev"
3940
},

‎src/Security/UserClassBuilder.php‎

Lines changed: 76 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Symfony\Bundle\MakerBundle\Util\ClassSourceManipulator;
1818
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
1919
use Symfony\Component\Security\Core\User\UserInterface;
20+
use Symfony\Component\Security\Http\Attribute\IsGrantedContext;
2021

2122
/**
2223
* Adds logic to implement UserInterface to an existing User class.
@@ -37,7 +38,13 @@ public function addUserInterfaceImplementation(ClassSourceManipulator $manipulat
3738

3839
$this->addPasswordImplementation($manipulator, $userClassConfig);
3940

40-
$this->addEraseCredentials($manipulator);
41+
if (class_exists(IsGrantedContext::class)) {
42+
$this->addSerialize($manipulator);
43+
}
44+
45+
if (method_exists(UserInterface::class, 'eraseCredentials')) {
46+
$this->addEraseCredentials($manipulator);
47+
}
4148
}
4249

4350
private function addPasswordImplementation(ClassSourceManipulator $manipulator, UserClassConfiguration $userClassConfig): void
@@ -245,17 +252,80 @@ private function addEraseCredentials(ClassSourceManipulator $manipulator): void
245252
$builder = $manipulator->createMethodBuilder(
246253
'eraseCredentials',
247254
'void',
248-
false,
249-
['@see UserInterface']
255+
false
250256
);
257+
$builder->addAttribute(new Node\Attribute(new Node\Name('\Deprecated')));
251258
$builder->addStmt(
252259
$manipulator->createMethodLevelCommentNode(
253-
'If you store any temporary, sensitive data on the user, clear it here'
260+
'@deprecated, to be removed when upgrading to Symfony 8'
254261
)
255262
);
263+
264+
$manipulator->addMethodBuilder($builder);
265+
}
266+
267+
private function addSerialize(ClassSourceManipulator $manipulator): void
268+
{
269+
$builder = $manipulator->createMethodBuilder(
270+
'__serialize',
271+
'array',
272+
false,
273+
[
274+
'Ensure the session doesn\'t contain actual password hashes by CRC32C-hashing them, as supported since Symfony 7.3.',
275+
]
276+
);
277+
278+
// $data = (array) $this;
256279
$builder->addStmt(
257-
$manipulator->createMethodLevelCommentNode(
258-
'$this->plainPassword = null;'
280+
new Node\Stmt\Expression(
281+
new Node\Expr\Assign(
282+
new Node\Expr\Variable('data'),
283+
new Node\Expr\Cast\Array_(
284+
new Node\Expr\Variable('this')
285+
)
286+
)
287+
)
288+
);
289+
290+
// $data["\0".self::class."\0password"] = hash('crc32c', $this->password);
291+
$builder->addStmt(
292+
new Node\Stmt\Expression(
293+
new Node\Expr\Assign(
294+
new Node\Expr\ArrayDimFetch(
295+
new Node\Expr\Variable('data'),
296+
new Node\Expr\BinaryOp\Concat(
297+
new Node\Expr\BinaryOp\Concat(
298+
new Node\Scalar\String_("\0", ['kind' => Node\Scalar\String_::KIND_DOUBLE_QUOTED]),
299+
new Node\Expr\ClassConstFetch(
300+
new Node\Name('self'),
301+
'class'
302+
)
303+
),
304+
new Node\Scalar\String_("\0password", ['kind' => Node\Scalar\String_::KIND_DOUBLE_QUOTED]),
305+
)
306+
),
307+
new Node\Expr\FuncCall(
308+
new Node\Name('hash'),
309+
[
310+
new Node\Arg(new Node\Scalar\String_('crc32c')),
311+
new Node\Arg(
312+
new Node\Expr\PropertyFetch(
313+
new Node\Expr\Variable('this'),
314+
'password'
315+
)
316+
),
317+
]
318+
)
319+
)
320+
)
321+
);
322+
323+
$builder->addStmt(new Node\Stmt\Nop());
324+
325+
// return $data;
326+
$builder->addStmt(
327+
new Node\Stmt\Return_(
328+
new Node\Expr\Variable('data')
259329
)
260330
);
261331

‎src/Util/PrettyPrinter.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ protected function pStmt_ClassMethod(Stmt\ClassMethod $node): string
5959
if ($node->returnType) {
6060
$classMethod = str_replace(') :', '):', $classMethod);
6161
}
62+
$classMethod = str_replace('\x00', '\0', $classMethod);
6263

6364
return $classMethod;
6465
}

‎tests/Security/UserClassBuilderTest.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
use Symfony\Bundle\MakerBundle\Security\UserClassBuilder;
1616
use Symfony\Bundle\MakerBundle\Security\UserClassConfiguration;
1717
use Symfony\Bundle\MakerBundle\Util\ClassSourceManipulator;
18+
use Symfony\Component\Security\Core\User\UserInterface;
19+
use Symfony\Component\Security\Http\Attribute\IsGrantedContext;
1820

1921
class UserClassBuilderTest extends TestCase
2022
{
@@ -31,6 +33,14 @@ public function testAddUserInterfaceImplementation(UserClassConfiguration $userC
3133
$expectedPath = $this->getExpectedPath($expectedFilename, null);
3234
$expectedSource = file_get_contents($expectedPath);
3335

36+
if (!class_exists(IsGrantedContext::class)) {
37+
$expectedSource = preg_replace('/\n\n(.+\n)+.+function __serialize[^}]++}/', '', $expectedSource);
38+
}
39+
40+
if (!method_exists(UserInterface::class, 'eraseCredentials')) {
41+
$expectedSource = preg_replace('/\n\n(.+\n)+.+function eraseCredentials[^}]++}/', '', $expectedSource);
42+
}
43+
3444
self::assertSame($expectedSource, $manipulator->getSourceCode());
3545
}
3646

‎tests/Security/fixtures/expected/UserEntityWithEmailAsIdentifier.php‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,19 @@ public function setPassword(string $password): static
9595
}
9696

9797
/**
98-
* @see UserInterface
98+
* Ensure the session doesn't contain actual password hashes by CRC32C-hashing them, as supported since Symfony 7.3.
9999
*/
100+
public function __serialize(): array
101+
{
102+
$data = (array) $this;
103+
$data["\0" . self::class . "\0password"] = hash('crc32c', $this->password);
104+
105+
return $data;
106+
}
107+
108+
#[\Deprecated]
100109
public function eraseCredentials(): void
101110
{
102-
// If you store any temporary, sensitive data on the user, clear it here
103-
// $this->plainPassword = null;
111+
// @deprecated, to be removed when upgrading to Symfony 8
104112
}
105113
}

‎tests/Security/fixtures/expected/UserEntityWithPassword.php‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,19 @@ public function setPassword(string $password): static
9090
}
9191

9292
/**
93-
* @see UserInterface
93+
* Ensure the session doesn't contain actual password hashes by CRC32C-hashing them, as supported since Symfony 7.3.
9494
*/
95+
public function __serialize(): array
96+
{
97+
$data = (array) $this;
98+
$data["\0" . self::class . "\0password"] = hash('crc32c', $this->password);
99+
100+
return $data;
101+
}
102+
103+
#[\Deprecated]
95104
public function eraseCredentials(): void
96105
{
97-
// If you store any temporary, sensitive data on the user, clear it here
98-
// $this->plainPassword = null;
106+
// @deprecated, to be removed when upgrading to Symfony 8
99107
}
100108
}

‎tests/Security/fixtures/expected/UserEntityWithUser_IdentifierAsIdentifier.php‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,19 @@ public function setPassword(string $password): static
9090
}
9191

9292
/**
93-
* @see UserInterface
93+
* Ensure the session doesn't contain actual password hashes by CRC32C-hashing them, as supported since Symfony 7.3.
9494
*/
95+
public function __serialize(): array
96+
{
97+
$data = (array) $this;
98+
$data["\0" . self::class . "\0password"] = hash('crc32c', $this->password);
99+
100+
return $data;
101+
}
102+
103+
#[\Deprecated]
95104
public function eraseCredentials(): void
96105
{
97-
// If you store any temporary, sensitive data on the user, clear it here
98-
// $this->plainPassword = null;
106+
// @deprecated, to be removed when upgrading to Symfony 8
99107
}
100108
}

‎tests/Security/fixtures/expected/UserEntityWithoutPassword.php‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,19 @@ public function setRoles(array $roles): static
6868
}
6969

7070
/**
71-
* @see UserInterface
71+
* Ensure the session doesn't contain actual password hashes by CRC32C-hashing them, as supported since Symfony 7.3.
7272
*/
73+
public function __serialize(): array
74+
{
75+
$data = (array) $this;
76+
$data["\0" . self::class . "\0password"] = hash('crc32c', $this->password);
77+
78+
return $data;
79+
}
80+
81+
#[\Deprecated]
7382
public function eraseCredentials(): void
7483
{
75-
// If you store any temporary, sensitive data on the user, clear it here
76-
// $this->plainPassword = null;
84+
// @deprecated, to be removed when upgrading to Symfony 8
7785
}
7886
}

‎tests/Security/fixtures/expected/UserModelWithEmailAsIdentifier.php‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,19 @@ public function setPassword(string $password): static
7979
}
8080

8181
/**
82-
* @see UserInterface
82+
* Ensure the session doesn't contain actual password hashes by CRC32C-hashing them, as supported since Symfony 7.3.
8383
*/
84+
public function __serialize(): array
85+
{
86+
$data = (array) $this;
87+
$data["\0" . self::class . "\0password"] = hash('crc32c', $this->password);
88+
89+
return $data;
90+
}
91+
92+
#[\Deprecated]
8493
public function eraseCredentials(): void
8594
{
86-
// If you store any temporary, sensitive data on the user, clear it here
87-
// $this->plainPassword = null;
95+
// @deprecated, to be removed when upgrading to Symfony 8
8896
}
8997
}

‎tests/Security/fixtures/expected/UserModelWithPassword.php‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,19 @@ public function setPassword(string $password): static
7474
}
7575

7676
/**
77-
* @see UserInterface
77+
* Ensure the session doesn't contain actual password hashes by CRC32C-hashing them, as supported since Symfony 7.3.
7878
*/
79+
public function __serialize(): array
80+
{
81+
$data = (array) $this;
82+
$data["\0" . self::class . "\0password"] = hash('crc32c', $this->password);
83+
84+
return $data;
85+
}
86+
87+
#[\Deprecated]
7988
public function eraseCredentials(): void
8089
{
81-
// If you store any temporary, sensitive data on the user, clear it here
82-
// $this->plainPassword = null;
90+
// @deprecated, to be removed when upgrading to Symfony 8
8391
}
8492
}

0 commit comments

Comments
 (0)