Skip to content

Commit 0d0ab4d

Browse files
Merge branch '7.1' into 7.2
* 7.1: [HttpClient] Resolve hostnames in NoPrivateNetworkHttpClient [security-http] Check owner of persisted remember-me cookie
2 parents dd89ea6 + e11ea7f commit 0d0ab4d

File tree

2 files changed

+48
-5
lines changed

2 files changed

+48
-5
lines changed

RememberMe/PersistentRememberMeHandler.php

+16-3
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,16 @@ public function consumeRememberMeCookie(RememberMeDetails $rememberMeDetails): U
6464
throw new AuthenticationException('The cookie is incorrectly formatted.');
6565
}
6666

67-
[$series, $tokenValue] = explode(':', $rememberMeDetails->getValue());
67+
[$series, $tokenValue] = explode(':', $rememberMeDetails->getValue(), 2);
6868
$persistentToken = $this->tokenProvider->loadTokenBySeries($series);
6969

70+
if ($persistentToken->getUserIdentifier() !== $rememberMeDetails->getUserIdentifier() || $persistentToken->getClass() !== $rememberMeDetails->getUserFqcn()) {
71+
throw new AuthenticationException('The cookie\'s hash is invalid.');
72+
}
73+
74+
// content of $rememberMeDetails is not trustable. this prevents use of this class
75+
unset($rememberMeDetails);
76+
7077
if ($this->tokenVerifier) {
7178
$isTokenValid = $this->tokenVerifier->verifyToken($persistentToken, $tokenValue);
7279
} else {
@@ -76,11 +83,17 @@ public function consumeRememberMeCookie(RememberMeDetails $rememberMeDetails): U
7683
throw new CookieTheftException('This token was already used. The account is possibly compromised.');
7784
}
7885

79-
if ($persistentToken->getLastUsed()->getTimestamp() + $this->options['lifetime'] < time()) {
86+
$expires = $persistentToken->getLastUsed()->getTimestamp() + $this->options['lifetime'];
87+
if ($expires < time()) {
8088
throw new AuthenticationException('The cookie has expired.');
8189
}
8290

83-
return parent::consumeRememberMeCookie($rememberMeDetails->withValue($persistentToken->getLastUsed()->getTimestamp().':'.$rememberMeDetails->getValue().':'.$persistentToken->getClass()));
91+
return parent::consumeRememberMeCookie(new RememberMeDetails(
92+
$persistentToken->getClass(),
93+
$persistentToken->getUserIdentifier(),
94+
$expires,
95+
$persistentToken->getLastUsed()->getTimestamp().':'.$series.':'.$tokenValue.':'.$persistentToken->getClass()
96+
));
8497
}
8598

8699
public function processRememberMe(RememberMeDetails $rememberMeDetails, UserInterface $user): void

Tests/RememberMe/PersistentRememberMeHandlerTest.php

+32-2
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public function testConsumeRememberMeCookieValid()
7979
$this->tokenProvider->expects($this->any())
8080
->method('loadTokenBySeries')
8181
->with('series1')
82-
->willReturn(new PersistentToken(InMemoryUser::class, 'wouter', 'series1', 'tokenvalue', new \DateTimeImmutable('-10 min')))
82+
->willReturn(new PersistentToken(InMemoryUser::class, 'wouter', 'series1', 'tokenvalue', $lastUsed = new \DateTimeImmutable('-10 min')))
8383
;
8484

8585
$this->tokenProvider->expects($this->once())->method('updateToken')->with('series1');
@@ -97,11 +97,41 @@ public function testConsumeRememberMeCookieValid()
9797

9898
$this->assertSame($rememberParts[0], $cookieParts[0]); // class
9999
$this->assertSame($rememberParts[1], $cookieParts[1]); // identifier
100-
$this->assertSame($rememberParts[2], $cookieParts[2]); // expire
100+
$this->assertEqualsWithDelta($lastUsed->getTimestamp() + 31536000, (int) $cookieParts[2], 2); // expire
101101
$this->assertNotSame($rememberParts[3], $cookieParts[3]); // value
102102
$this->assertSame(explode(':', $rememberParts[3])[0], explode(':', $cookieParts[3])[0]); // series
103103
}
104104

105+
public function testConsumeRememberMeCookieInvalidOwner()
106+
{
107+
$this->tokenProvider->expects($this->any())
108+
->method('loadTokenBySeries')
109+
->with('series1')
110+
->willReturn(new PersistentToken(InMemoryUser::class, 'wouter', 'series1', 'tokenvalue', new \DateTime('-10 min')))
111+
;
112+
113+
$rememberMeDetails = new RememberMeDetails(InMemoryUser::class, 'jeremy', 360, 'series1:tokenvalue');
114+
115+
$this->expectException(AuthenticationException::class);
116+
$this->expectExceptionMessage('The cookie\'s hash is invalid.');
117+
$this->handler->consumeRememberMeCookie($rememberMeDetails);
118+
}
119+
120+
public function testConsumeRememberMeCookieInvalidValue()
121+
{
122+
$this->tokenProvider->expects($this->any())
123+
->method('loadTokenBySeries')
124+
->with('series1')
125+
->willReturn(new PersistentToken(InMemoryUser::class, 'wouter', 'series1', 'tokenvalue', new \DateTime('-10 min')))
126+
;
127+
128+
$rememberMeDetails = new RememberMeDetails(InMemoryUser::class, 'wouter', 360, 'series1:tokenvalue:somethingelse');
129+
130+
$this->expectException(AuthenticationException::class);
131+
$this->expectExceptionMessage('This token was already used. The account is possibly compromised.');
132+
$this->handler->consumeRememberMeCookie($rememberMeDetails);
133+
}
134+
105135
public function testConsumeRememberMeCookieValidByValidatorWithoutUpdate()
106136
{
107137
$verifier = $this->createMock(TokenVerifierInterface::class);

0 commit comments

Comments
 (0)