diff options
author | VirtualTam <virtualtam@flibidi.net> | 2018-05-10 13:07:51 +0200 |
---|---|---|
committer | VirtualTam <virtualtam@flibidi.net> | 2018-06-02 16:46:06 +0200 |
commit | ebf615173824a46de82fa97a165bcfd883db15ce (patch) | |
tree | 26374298b3c7f2009ef939c5d5e3d787938581be | |
parent | c689e108639a4f6aa9e15928422e14db7cbe30ca (diff) | |
download | Shaarli-ebf615173824a46de82fa97a165bcfd883db15ce.tar.gz Shaarli-ebf615173824a46de82fa97a165bcfd883db15ce.tar.zst Shaarli-ebf615173824a46de82fa97a165bcfd883db15ce.zip |
SessionManager: remove unused UID token
There already are dedicated tokens for:
- CSRF protection
- user stay-signed-in feature, via cookie
This token was most likely intended as a randomly generated,
server-side, secret key to be used when generating hashes.
See http://sebsauvage.net/wiki/doku.php?id=php:session [FR]
Relevant section:
Une clé secrète unique aléatoire est générée côté serveur (et jamais
envoyée). Elle peut servir pour signer les formulaires (HMAC) ou
générer des token de formulaires (protection contre XSRF).
Voir $_SESSION['uid'].
Translation:
A unique, server-side secret key is randomly generated (and never
transmitted). It can be used to sign forms (HMAC) or generate form
tokens (protection against XSRF).
See $_SESSION['uid']
Signed-off-by: VirtualTam <virtualtam@flibidi.net>
-rw-r--r-- | application/security/SessionManager.php | 6 | ||||
-rw-r--r-- | tests/security/SessionManagerTest.php | 13 |
2 files changed, 0 insertions, 19 deletions
diff --git a/application/security/SessionManager.php b/application/security/SessionManager.php index 58973130..24e25528 100644 --- a/application/security/SessionManager.php +++ b/application/security/SessionManager.php | |||
@@ -113,8 +113,6 @@ class SessionManager | |||
113 | */ | 113 | */ |
114 | public function storeLoginInfo($clientIpId) | 114 | public function storeLoginInfo($clientIpId) |
115 | { | 115 | { |
116 | // Generate unique random number (different than phpsessionid) | ||
117 | $this->session['uid'] = sha1(uniqid('', true) . '_' . mt_rand()); | ||
118 | $this->session['ip'] = $clientIpId; | 116 | $this->session['ip'] = $clientIpId; |
119 | $this->session['username'] = $this->conf->get('credentials.login'); | 117 | $this->session['username'] = $this->conf->get('credentials.login'); |
120 | $this->extendTimeValidityBy(self::$SHORT_TIMEOUT); | 118 | $this->extendTimeValidityBy(self::$SHORT_TIMEOUT); |
@@ -154,7 +152,6 @@ class SessionManager | |||
154 | public function logout() | 152 | public function logout() |
155 | { | 153 | { |
156 | if (isset($this->session)) { | 154 | if (isset($this->session)) { |
157 | unset($this->session['uid']); | ||
158 | unset($this->session['ip']); | 155 | unset($this->session['ip']); |
159 | unset($this->session['expires_on']); | 156 | unset($this->session['expires_on']); |
160 | unset($this->session['username']); | 157 | unset($this->session['username']); |
@@ -172,9 +169,6 @@ class SessionManager | |||
172 | */ | 169 | */ |
173 | public function hasSessionExpired() | 170 | public function hasSessionExpired() |
174 | { | 171 | { |
175 | if (empty($this->session['uid'])) { | ||
176 | return true; | ||
177 | } | ||
178 | if (time() >= $this->session['expires_on']) { | 172 | if (time() >= $this->session['expires_on']) { |
179 | return true; | 173 | return true; |
180 | } | 174 | } |
diff --git a/tests/security/SessionManagerTest.php b/tests/security/SessionManagerTest.php index e1c72707..ae10ffa6 100644 --- a/tests/security/SessionManagerTest.php +++ b/tests/security/SessionManagerTest.php | |||
@@ -164,7 +164,6 @@ class SessionManagerTest extends TestCase | |||
164 | { | 164 | { |
165 | $this->sessionManager->storeLoginInfo('ip_id'); | 165 | $this->sessionManager->storeLoginInfo('ip_id'); |
166 | 166 | ||
167 | $this->assertTrue(isset($this->session['uid'])); | ||
168 | $this->assertGreaterThan(time(), $this->session['expires_on']); | 167 | $this->assertGreaterThan(time(), $this->session['expires_on']); |
169 | $this->assertEquals('ip_id', $this->session['ip']); | 168 | $this->assertEquals('ip_id', $this->session['ip']); |
170 | $this->assertEquals('johndoe', $this->session['username']); | 169 | $this->assertEquals('johndoe', $this->session['username']); |
@@ -209,7 +208,6 @@ class SessionManagerTest extends TestCase | |||
209 | public function testLogout() | 208 | public function testLogout() |
210 | { | 209 | { |
211 | $this->session = [ | 210 | $this->session = [ |
212 | 'uid' => 'some-uid', | ||
213 | 'ip' => 'ip_id', | 211 | 'ip' => 'ip_id', |
214 | 'expires_on' => time() + 1000, | 212 | 'expires_on' => time() + 1000, |
215 | 'username' => 'johndoe', | 213 | 'username' => 'johndoe', |
@@ -218,7 +216,6 @@ class SessionManagerTest extends TestCase | |||
218 | ]; | 216 | ]; |
219 | $this->sessionManager->logout(); | 217 | $this->sessionManager->logout(); |
220 | 218 | ||
221 | $this->assertFalse(isset($this->session['uid'])); | ||
222 | $this->assertFalse(isset($this->session['ip'])); | 219 | $this->assertFalse(isset($this->session['ip'])); |
223 | $this->assertFalse(isset($this->session['expires_on'])); | 220 | $this->assertFalse(isset($this->session['expires_on'])); |
224 | $this->assertFalse(isset($this->session['username'])); | 221 | $this->assertFalse(isset($this->session['username'])); |
@@ -227,19 +224,10 @@ class SessionManagerTest extends TestCase | |||
227 | } | 224 | } |
228 | 225 | ||
229 | /** | 226 | /** |
230 | * The session is considered as expired because the UID is missing | ||
231 | */ | ||
232 | public function testHasExpiredNoUid() | ||
233 | { | ||
234 | $this->assertTrue($this->sessionManager->hasSessionExpired()); | ||
235 | } | ||
236 | |||
237 | /** | ||
238 | * The session is active and expiration time has been reached | 227 | * The session is active and expiration time has been reached |
239 | */ | 228 | */ |
240 | public function testHasExpiredTimeElapsed() | 229 | public function testHasExpiredTimeElapsed() |
241 | { | 230 | { |
242 | $this->session['uid'] = 'some-uid'; | ||
243 | $this->session['expires_on'] = time() - 10; | 231 | $this->session['expires_on'] = time() - 10; |
244 | 232 | ||
245 | $this->assertTrue($this->sessionManager->hasSessionExpired()); | 233 | $this->assertTrue($this->sessionManager->hasSessionExpired()); |
@@ -250,7 +238,6 @@ class SessionManagerTest extends TestCase | |||
250 | */ | 238 | */ |
251 | public function testHasNotExpired() | 239 | public function testHasNotExpired() |
252 | { | 240 | { |
253 | $this->session['uid'] = 'some-uid'; | ||
254 | $this->session['expires_on'] = time() + 1000; | 241 | $this->session['expires_on'] = time() + 1000; |
255 | 242 | ||
256 | $this->assertFalse($this->sessionManager->hasSessionExpired()); | 243 | $this->assertFalse($this->sessionManager->hasSessionExpired()); |