Merge pull request #186 from akrabat/Accepting-Headers

Change key names so that they are valid header names
slimphp · Jun 8, 2024 · 56a0cfb · 56a0cfb
2 parents 179cbcf + 23e2c75
commit 56a0cfb
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 53 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,7 +2,13 @@
 
 See https://github.com/slimphp/Slim-Csrf/releases for a full list
 
-## Next
+## 2.x
+
+- Fixed: The key names are now the correct format to be sent as headers. 
+
+  This is a potential BC break. The key names now use a dash rather than an
+  underscore. This should not affect anyone who uses the relvant methods, but
+  if you have hard-coded, then they will need to be updated.
 
 ## 1.5.0
 

diff --git a/src/Guard.php b/src/Guard.php
@@ -50,7 +50,7 @@ class Guard implements MiddlewareInterface
     protected ResponseFactoryInterface $responseFactory;
 
     /**
-     * Prefix for CSRF parameters (omit trailing "_" underscore)
+     * Prefix for CSRF parameters (omit trailing "-")
      */
     protected string $prefix;
 
@@ -122,7 +122,7 @@ public function __construct(
         }
 
         $this->responseFactory = $responseFactory;
-        $this->prefix = rtrim($prefix, '_');
+        $this->prefix = rtrim($prefix, '-');
         $this->strength = $strength;
 
         $this->setStorage($storage);
@@ -269,15 +269,15 @@ public function getTokenValue(): ?string
      */
     public function getTokenNameKey(): string
     {
-        return $this->prefix . '_name';
+        return $this->prefix . '-name';
     }
 
     /**
      * @return string
      */
     public function getTokenValueKey(): string
     {
-        return $this->prefix . '_value';
+        return $this->prefix . '-value';
     }
 
     /**

diff --git a/tests/GuardTest.php b/tests/GuardTest.php
@@ -193,35 +193,35 @@ public function testDefaultFailureHandler()
     public function testValidateToken()
     {
         $storage = [
-            'test_name' => 'value'
+            'test-name' => 'value'
         ];
         $responseFactoryProphecy = $this->prophesize(ResponseFactoryInterface::class);
         $mw = new Guard($responseFactoryProphecy->reveal(), 'test', $storage);
 
         $maskedToken = $this->maskToken($mw, 'value');
-        $this->assertTrue($mw->validateToken('test_name', $maskedToken));
+        $this->assertTrue($mw->validateToken('test-name', $maskedToken));
 
         $maskedToken2 = $this->maskToken($mw, 'value');
-        $this->assertTrue($mw->validateToken('test_name', $maskedToken2));
+        $this->assertTrue($mw->validateToken('test-name', $maskedToken2));
 
         $this->assertNotSame($maskedToken, $maskedToken2);
     }
 
     public function testNotValidatingBadToken()
     {
         $storage = [
-            'test_name' => 'value'
+            'test-name' => 'value'
         ];
         $responseFactoryProphecy = $this->prophesize(ResponseFactoryInterface::class);
         $mw = new Guard($responseFactoryProphecy->reveal(), 'test', $storage);
 
         $maskedToken = 'MY_BAD_BASE64???';
-        $this->assertFalse($mw->validateToken('test_name', $maskedToken), 'Token contains bad base64 characters');
+        $this->assertFalse($mw->validateToken('test-name', $maskedToken), 'Token contains bad base64 characters');
 
         $maskedToken2 = $this->maskToken($mw, 'value');
         // Remove some part of base64
         $maskedToken2 = substr($maskedToken2, 0, -6);
-        $this->assertFalse($mw->validateToken('test_name', $maskedToken2), 'Token size should be even');
+        $this->assertFalse($mw->validateToken('test-name', $maskedToken2), 'Token size should be even');
     }
 
     public function testGetTokenNameAndValue()
@@ -238,12 +238,12 @@ public function testGetTokenNameAndValue()
         $loadLastKeyPairMethod->invoke($mw);
 
         $storage = [
-            'test_name' => 'value',
+            'test-name' => 'value',
         ];
         $mw->setStorage($storage);
         $loadLastKeyPairMethod->invoke($mw);
 
-        $this->assertEquals('test_name', $mw->getTokenName());
+        $this->assertEquals('test-name', $mw->getTokenName());
 
         $unmaskTokenMethod = new ReflectionMethod($mw, 'unmaskToken');
         $unmaskTokenMethod->setAccessible(true);
@@ -266,30 +266,30 @@ public function testGetTokenNameKeyAndValue()
         $responseFactoryProphecy = $this->prophesize(ResponseFactoryInterface::class);
         $mw = new Guard($responseFactoryProphecy->reveal(), 'test', $storage);
 
-        $this->assertEquals('test_name', $mw->getTokenNameKey());
-        $this->assertEquals('test_value', $mw->getTokenValueKey());
+        $this->assertEquals('test-name', $mw->getTokenNameKey());
+        $this->assertEquals('test-value', $mw->getTokenValueKey());
     }
 
     public function testRemoveTokenFromStorage()
     {
         $storage = [
-            'test_name' => 'value',
+            'test-name' => 'value',
         ];
         $responseFactoryProphecy = $this->prophesize(ResponseFactoryInterface::class);
         $mw = new Guard($responseFactoryProphecy->reveal(), 'test', $storage);
 
         $removeTokenFromStorageMethod = new ReflectionMethod($mw, 'removeTokenFromStorage');
         $removeTokenFromStorageMethod->setAccessible(true);
-        $removeTokenFromStorageMethod->invoke($mw, 'test_name');
+        $removeTokenFromStorageMethod->invoke($mw, 'test-name');
 
-        $this->assertArrayNotHasKey('test_name', $storage);
+        $this->assertArrayNotHasKey('test-name', $storage);
     }
 
     public function testEnforceStorageLimitWithArray()
     {
         $storage = [
-            'test_name' => 'value',
-            'test_name2' => 'value2',
+            'test-name' => 'value',
+            'test-name2' => 'value2',
         ];
         $responseFactoryProphecy = $this->prophesize(ResponseFactoryInterface::class);
         $mw = new Guard($responseFactoryProphecy->reveal(), 'test', $storage, null, 1);
@@ -298,15 +298,15 @@ public function testEnforceStorageLimitWithArray()
         $enforceStorageLimitMethod->setAccessible(true);
         $enforceStorageLimitMethod->invoke($mw);
 
-        $this->assertArrayNotHasKey('test_name', $storage);
-        $this->assertArrayHasKey('test_name2', $storage);
+        $this->assertArrayNotHasKey('test-name', $storage);
+        $this->assertArrayHasKey('test-name2', $storage);
     }
 
     public function testNotEnforceStorageLimitWithArrayWhenLimitIsZero()
     {
         $initial_storage = $storage = [
-            'test_name' => 'value',
-            'test_name2' => 'value2',
+            'test-name' => 'value',
+            'test-name2' => 'value2',
         ];
         $responseFactoryProphecy = $this->prophesize(ResponseFactoryInterface::class);
         $mw = new Guard($responseFactoryProphecy->reveal(), 'test', $storage, null, 0);
@@ -321,8 +321,8 @@ public function testNotEnforceStorageLimitWithArrayWhenLimitIsZero()
     public function testEnforceStorageLimitWithIterator()
     {
         $storage = new ArrayIterator([
-            'test_name' => 'value',
-            'test_name2' => 'value',
+            'test-name' => 'value',
+            'test-name2' => 'value',
         ]);
         $responseFactoryProphecy = $this->prophesize(ResponseFactoryInterface::class);
         $mw = new Guard($responseFactoryProphecy->reveal(), 'test', $storage, null, 1);
@@ -331,14 +331,14 @@ public function testEnforceStorageLimitWithIterator()
         $enforceStorageLimitMethod->setAccessible(true);
         $enforceStorageLimitMethod->invoke($mw);
 
-        $this->assertArrayNotHasKey('test_name', $storage);
-        $this->assertArrayHasKey('test_name2', $storage);
+        $this->assertArrayNotHasKey('test-name', $storage);
+        $this->assertArrayHasKey('test-name2', $storage);
     }
 
     public function testTokenIsRemovedFromStorageWhenPersistentModeIsOff()
     {
         $storage = [
-            'test_name' => 'test_value123',
+            'test-name' => 'test-value123',
         ];
 
         $responseProphecy = $this->prophesize(ResponseInterface::class)
@@ -366,21 +366,21 @@ public function testTokenIsRemovedFromStorageWhenPersistentModeIsOff()
         $requestProphecy
             ->getParsedBody()
             ->willReturn([
-                'test_name' => 'test_name',
-                'test_value' => $this->maskToken($mw, 'test_value123'),
+                'test-name' => 'test-name',
+                'test-value' => $this->maskToken($mw, 'test-value123'),
             ])
             ->shouldBeCalledOnce();
 
 
         $mw->process($requestProphecy->reveal(), $requestHandlerProphecy->reveal());
-        self::assertArrayNotHasKey('test_name', $storage);
+        self::assertArrayNotHasKey('test-name', $storage);
     }
 
     public function testTokenIsRemovedFromStorageWhenPersistentModeIsOffOnFailure()
     {
         $storage = [
-            'test_name' => 'test_value123',
-            'test_name2' => 'test_value234',
+            'test-name' => 'test-value123',
+            'test-name2' => 'test-value234',
         ];
 
         $streamProphecy = $this->prophesize(StreamInterface::class);
@@ -429,19 +429,19 @@ public function testTokenIsRemovedFromStorageWhenPersistentModeIsOffOnFailure()
         $requestProphecy
             ->getParsedBody()
             ->willReturn([
-                             'test_name' => 'test_value123',
+                             'test-name' => 'test-value123',
                          ])
             ->shouldBeCalledOnce();
 
         $mw->process($requestProphecy->reveal(), $requestHandlerProphecy->reveal());
 
-        $this->assertArrayNotHasKey('test_name', $storage);
+        $this->assertArrayNotHasKey('test-name', $storage);
     }
 
     public function testTokenInBodyOfGetIsInvalid()
     {
         $storage = [
-            'test_name' => 'test_value123',
+            'test-name' => 'test-value123',
         ];
 
         // we set up a failure handler that we expect to be called because a GET cannot have a token
@@ -467,8 +467,8 @@ public function testTokenInBodyOfGetIsInvalid()
         $requestProphecy
             ->getParsedBody()
             ->willReturn([
-                'test_name' => 'test_name',
-                'test_value' => 'test_value123',
+                'test-name' => 'test-name',
+                'test-value' => 'test-value123',
             ])
             ->shouldBeCalledOnce();
 
@@ -510,7 +510,7 @@ public function testProcessAppendsNewTokensWhenPersistentTokenModeIsOff()
     public function testProcessAppendsNewTokensWhenPersistentTokenModeIsOn()
     {
         $storage = [
-            'test_name123' => 'test_value123',
+            'test-name123' => 'test-value123',
         ];
         $responseFactoryProphecy = $this->prophesize(ResponseFactoryInterface::class);
         $mw = new Guard($responseFactoryProphecy->reveal(), 'test', $storage, null, 200, 16, true);
@@ -526,12 +526,12 @@ public function testProcessAppendsNewTokensWhenPersistentTokenModeIsOn()
             ->shouldBeCalledOnce();
 
         $requestProphecy
-            ->withAttribute('test_name', 'test_name123')
+            ->withAttribute('test-name', 'test-name123')
             ->willReturn($requestProphecy->reveal())
             ->shouldBeCalledOnce();
 
         $requestProphecy
-            ->withAttribute('test_value', Argument::type('string'))
+            ->withAttribute('test-value', Argument::type('string'))
             ->willReturn($requestProphecy->reveal())
             ->shouldBeCalledOnce();
 
@@ -548,8 +548,8 @@ public function testProcessAppendsNewTokensWhenPersistentTokenModeIsOn()
     public function testCanGetLastKeyPairFromIterator()
     {
         $storage = new ArrayIterator([
-            'test_key1' => 'value1',
-            'test_key2' => 'value2',
+            'test-key1' => 'value1',
+            'test-key2' => 'value2',
         ]);
         $responseFactoryProphecy = $this->prophesize(ResponseFactoryInterface::class);
         $mw = new Guard($responseFactoryProphecy->reveal(), 'test', $storage, null, 1);
@@ -559,20 +559,20 @@ public function testCanGetLastKeyPairFromIterator()
         $keyPair = $enforceStorageLimitMethod->invoke($mw);
 
         $this->assertIsArray($keyPair);
-        $this->assertArrayHasKey('test_name', $keyPair);
-        $this->assertArrayHasKey('test_value', $keyPair);
-        $this->assertEquals('test_key2', $keyPair['test_name']);
+        $this->assertArrayHasKey('test-name', $keyPair);
+        $this->assertArrayHasKey('test-value', $keyPair);
+        $this->assertEquals('test-key2', $keyPair['test-name']);
 
         $unmaskTokenMethod = new ReflectionMethod($mw, 'unmaskToken');
         $unmaskTokenMethod->setAccessible(true);
-        $unmaskedToken = $unmaskTokenMethod->invoke($mw, $keyPair['test_value']);
+        $unmaskedToken = $unmaskTokenMethod->invoke($mw, $keyPair['test-value']);
         $this->assertEquals('value2', $unmaskedToken);
     }
 
     public function testTokenFromHeaderOnDelete()
     {
         $storage = [
-            'test_name' => 'test_value123',
+            'test-name' => 'test-value123',
         ];
 
         $responseProphecy = $this->prophesize(ResponseInterface::class)
@@ -602,12 +602,12 @@ public function testTokenFromHeaderOnDelete()
             ->willReturn([])
             ->shouldBeCalledOnce();
         $requestProphecy
-            ->getHeader('test_name')
-            ->willReturn(['test_name'])
+            ->getHeader('test-name')
+            ->willReturn(['test-name'])
             ->shouldBeCalledOnce();
         $requestProphecy
-            ->getHeader('test_value')
-            ->willReturn([$this->maskToken($mw, 'test_value123')])
+            ->getHeader('test-value')
+            ->willReturn([$this->maskToken($mw, 'test-value123')])
             ->shouldBeCalledOnce();
 
         $mw->process($requestProphecy->reveal(), $requestHandlerProphecy->reveal());