Skip to content

Commit 5bf84fb

Browse files
nicolas-grekasnicolas-grekas
committed
[make:user] Hash passwords using crc32c and deprecate eraseCredentials()
1 parent 468ff27 commit 5bf84fb

11 files changed

+165
-27
lines changed

‎composer.json

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

‎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
@@ -96,11 +96,19 @@ public function setPassword(string $password): static
9696
}
9797

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

5656
/**
57-
* @see UserInterface
57+
* Ensure the session doesn't contain actual password hashes by CRC32C-hashing them, as supported since Symfony 7.3.
5858
*/
59+
public function __serialize(): array
60+
{
61+
$data = (array) $this;
62+
$data["\0" . self::class . "\0password"] = hash('crc32c', $this->password);
63+
64+
return $data;
65+
}
66+
67+
#[\Deprecated]
5968
public function eraseCredentials(): void
6069
{
61-
// If you store any temporary, sensitive data on the user, clear it here
62-
// $this->plainPassword = null;
70+
// @deprecated, to be removed when upgrading to Symfony 8
6371
}
6472
}

0 commit comments

Comments
 (0)