Token is not removed after next protection

klapuch · Nov 17, 2016 · 3ea9613 · 3ea9613
1 parent 5217ac6
commit 3ea9613
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 8 deletions.
diff --git a/Core/StoredCsrf.php b/Core/StoredCsrf.php
@@ -19,7 +19,7 @@ public function __construct(array &$session, array $post, array $get) {
 	}
 
 	public function protection(): string {
-		return $this->session[self::NAME] = $this->token();
+		return $this->session[self::NAME] = $this->session[self::NAME] ?? $this->token();
 	}
 
 	public function abused(): bool {

diff --git a/Tests/Unit/StoredCsrf.phpt b/Tests/Unit/StoredCsrf.phpt
@@ -31,11 +31,14 @@ final class StoredCsrf extends Tester\TestCase {
 		Assert::true(strlen($protection) >= 20);
 	}
 
-	public function testGeneratingMultipleDifferentProtections() {
+	public function testGeneratingMultipleProtectionsWithoutOverwriting() {
 		$csrf = new Csrf\StoredCsrf($this->session, $this->post, $this->get);
-		$first = $csrf->protection();
-		$second = $csrf->protection();
-		Assert::notSame($first, $second);
+		$oldProtection = $csrf->protection();
+		$oldSession = $this->session;
+		$newProtection = $csrf->protection();
+		$newSession = $this->session;
+		Assert::same($oldProtection, $newProtection);
+		Assert::same($oldSession, $newSession);
 	}
 
 	public function testStoringProtection() {
@@ -112,6 +115,8 @@ final class StoredCsrf extends Tester\TestCase {
 
 	public function testInsufficientProtectionInSession() {
 		$this->session[Csrf\Csrf::NAME] = 'abc0';
+		$this->post[Csrf\Csrf::NAME] = 'abc0';
+		$this->get[Csrf\Csrf::NAME] = 'abc0';
 		$csrf = new Csrf\StoredCsrf($this->session, $this->post, $this->get);
 		Assert::true($csrf->abused());
 	}
@@ -131,7 +136,7 @@ final class StoredCsrf extends Tester\TestCase {
 		Assert::false($csrf->abused());
 	}
 
-	public function testRestartingSessionAfterProperProtection() {
+	public function testClearingSessionAfterProperProtection() {
 		$this->session[Csrf\Csrf::NAME] = str_repeat('a', 22);
 		$this->get[Csrf\Csrf::NAME] = str_repeat('a', 22);
 		$csrf = new Csrf\StoredCsrf($this->session, $this->post, $this->get);
@@ -140,14 +145,37 @@ final class StoredCsrf extends Tester\TestCase {
 		Assert::count(0, $this->session);
 	}
 
-	public function testRestartingSessionAfterAbusing() {
+	public function testClearingSessionAfterAbusing() {
 		$this->session[Csrf\Csrf::NAME] = str_repeat('a', 22);
 		$this->get[Csrf\Csrf::NAME] = str_repeat('b', 22);
 		$csrf = new Csrf\StoredCsrf($this->session, $this->post, $this->get);
 		Assert::count(1, $this->session);
 		Assert::true($csrf->abused());
 		Assert::count(0, $this->session);
 	}
+
+	public function testClearingProtectedSessionsWithoutAffectingOthers() {
+		$this->session['foo'] = 'bar';
+		$csrf = new Csrf\StoredCsrf($this->session, $this->post, $this->get);
+		$csrf->protection();
+		Assert::count(2, $this->session);
+		$csrf->abused();
+		Assert::count(1, $this->session);
+		Assert::contains('bar', $this->session);
+	}
+
+	public function testNewProtectionAfterAbusing() {
+		$csrf = new Csrf\StoredCsrf($this->session, $this->post, $this->get);
+		$oldProtection = $csrf->protection();
+		$oldSession = $this->session;
+		$csrf->abused();
+		$newProtection = $csrf->protection();
+		$newSession = $this->session;
+		Assert::notSame($oldProtection, $newProtection);
+		Assert::count(1, $newSession);
+		Assert::count(1, $oldSession);
+		Assert::notSame($oldSession, $newSession);
+	}
 }
 
-(new StoredCsrf())->run();
+(new StoredCsrf())->run();