Skip to content

Commit

Permalink
Merge branch 'master' into mark-jwt-with-sensitiveparameter
Browse files Browse the repository at this point in the history
  • Loading branch information
Firehed committed Jul 19, 2024
2 parents 6fbbb93 + 2234efd commit ce3dcb3
Show file tree
Hide file tree
Showing 10 changed files with 68 additions and 97 deletions.
2 changes: 0 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ jobs:
- 'high'
- 'low'
php:
- '7.4'
- '8.0'
- '8.1'
- '8.2'
- '8.3'
Expand Down
4 changes: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@
}
},
"require": {
"php": "^7.4 || ^8.0",
"php": "^8.1",
"firehed/security": "^1.0"
},
"require-dev": {
"phpstan/phpstan": "^1.2",
"phpstan/phpstan-phpunit": "^1.0",
"phpstan/phpstan-strict-rules": "^1.0",
"phpunit/phpunit": "^9.0",
"phpunit/phpunit": "^10.5 || ^11.0",
"squizlabs/php_codesniffer": "^3.4"
},
"config": {
Expand Down
37 changes: 15 additions & 22 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
@@ -1,24 +1,17 @@
<?xml version="1.0" encoding="UTF-8"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/7.0/phpunit.xsd"
bootstrap="vendor/autoload.php"
forceCoversAnnotation="true"
backupGlobals="true"
beStrictAboutCoversAnnotation="false"
beStrictAboutOutputDuringTests="true"
beStrictAboutTodoAnnotatedTests="true"
verbose="true">
<testsuite name="default">
<directory suffix="Test.php">tests</directory>
</testsuite>

<filter>
<whitelist processUncoveredFilesFromWhitelist="true">
<directory suffix=".php">src</directory>
</whitelist>
</filter>

<logging>
<log type="coverage-clover" target="build/logs/clover.xml" />
</logging>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/11.2/phpunit.xsd" bootstrap="vendor/autoload.php" backupGlobals="true" beStrictAboutOutputDuringTests="true" cacheDirectory=".phpunit.cache" requireCoverageMetadata="true" beStrictAboutCoverageMetadata="false">
<coverage>
<report>
<clover outputFile="build/logs/clover.xml"/>
</report>
</coverage>
<testsuite name="default">
<directory suffix="Test.php">tests</directory>
</testsuite>
<logging/>
<source>
<include>
<directory suffix=".php">src</directory>
</include>
</source>
</phpunit>
15 changes: 5 additions & 10 deletions src/JWT.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ class JWT
* kid?: array-key,
* }
*/
private $headers = [
private array $headers = [
Header::ALGORITHM => null,
Header::TYPE => 'JWT',
];

/** @var array<mixed> */
private $claims = [];
private array $claims = [];

private string $signature;

Expand All @@ -44,10 +44,9 @@ public function __construct(array $claims = [])
$this->is_verified = true;
} // __construct

/** @param int|string $keyId */
public function getEncoded($keyId = null): string
public function getEncoded(int|string|null $keyId = null): string
{
list($alg, $secret, $id) = $this->keys->getKey($keyId);
[$alg, $secret, $id] = $this->keys->getKey($keyId);
$this->headers[Header::ALGORITHM] = $alg;
$this->headers[Header::KEY_ID] = $id;

Expand Down Expand Up @@ -136,8 +135,7 @@ private function authenticate(): void
}
}

/** @return int|string|null */
public function getKeyID()
public function getKeyID(): int|string|null
{
return $this->headers[Header::KEY_ID] ?? null;
} // getKeyID
Expand Down Expand Up @@ -167,9 +165,6 @@ private function sign(Secret $key): string
throw new Exception("Unsupported algorithm");
// use openssl_sign and friends to do the signing
}
if ($data === false) { // @phpstan-ignore-line this is valid in PHP<=7.4
throw new UnexpectedValueException('Payload could not be hashed');
}
return rtrim(strtr(base64_encode($data), '+/', '-_'), '=');
} // sign

Expand Down
21 changes: 7 additions & 14 deletions src/KeyContainer.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,39 +8,32 @@ class KeyContainer
{

/** @var array{Algorithm::*, Secret}[] */
private $keys = [];
private array $keys = [];

/** @var int|string|null */
private $default;
private int|string|null $default = null;

/** @var int|string|null */
private $last;
private int|string|null $last = null;

/**
* @param Algorithm::* $alg
* @param array-key $id
*/
public function addKey($id, string $alg, Secret $secret): self
public function addKey(int|string $id, string $alg, Secret $secret): self
{
$this->keys[$id] = [$alg, $secret];
$this->last = $id;
return $this;
}

/**
* @param array-key $id
*/
public function setDefaultKey($id): self
public function setDefaultKey(int|string $id): self
{
$this->default = $id;
return $this;
}

/**
* @param ?array-key $id Key ID
* @return array{Algorithm::*, Secret, string|int}
*/
public function getKey($id = null): array
public function getKey(int|string|null $id = null): array
{
// Prefer explicitly requested > explicit default > most recently added
$id = $id ?? $this->default ?? $this->last;
Expand All @@ -49,7 +42,7 @@ public function getKey($id = null): array
"No key found with id '$id'"
);
}
list($alg, $secret) = $this->keys[$id];
[$alg, $secret] = $this->keys[$id];
return [$alg, $secret, $id];
}
}
25 changes: 7 additions & 18 deletions src/SessionHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@ class SessionHandler implements SessionHandlerInterface
const CLAIM = 'sd';
const DEFAULT_COOKIE = 'jwt_sid';

/** @var string */
private $cookie = self::DEFAULT_COOKIE;
private string $cookie = self::DEFAULT_COOKIE;

/** @var KeyContainer */
private $secrets;
private KeyContainer $secrets;

/** @var callable */
private $writer = 'setcookie';
Expand All @@ -40,30 +38,24 @@ public function close(): bool
return true;
}

/**
* @param string $session_id
*/
public function destroy($session_id): bool
public function destroy(string $session_id): bool
{
($this->writer)($this->cookie, '', time()-86400); // Expire yesterday
return true;
}

/**
* No-op, interface adherence only
* @param int $maxlifetime
*/
public function gc($maxlifetime): int
public function gc(int $maxlifetime): int
{
return 0;
}

/**
* No-op, interface adherence only
* @param string $save_path
* @param string $name
*/
public function open($save_path, $name): bool
public function open(string $save_path, string $name): bool
{
return true;
}
Expand All @@ -73,11 +65,10 @@ public function open($save_path, $name): bool
* returns the data to be natively unserialized into the $_SESSION
* superglobal
*
* @param string $session_id (unused)
* @return string the serialized session string
* @throws JWTException if JWT processing fails, tampering is detected, etc
*/
public function read($session_id): string
public function read(string $session_id): string
{
// session_id is intentionally ignored
if (!array_key_exists($this->cookie, $_COOKIE)) {
Expand All @@ -99,12 +90,10 @@ public function read($session_id): string
/**
* Writes the session data to a cookie containing a signed JWT
*
* @param string $session_id (unused)
* @param string $session_data the serialized session data
* @throws OverflowException if there is too much session data
* @throws JWTException if the data cannot be signed
*/
public function write($session_id, $session_data): bool
public function write(string $session_id, string $session_data): bool
{
$data = [
Claim::JWT_ID => $session_id,
Expand Down
7 changes: 4 additions & 3 deletions tests/CodecTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@

use Firehed\Security\Secret;
use PHPUnit\Framework\TestCase;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\Small;

/**
* @covers Codec
*/
#[CoversClass(Codec::class)]
#[Small]
class CodecTest extends TestCase
{
private Codec $codec;
Expand Down
22 changes: 12 additions & 10 deletions tests/JWTTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,21 @@
use Error;
use Firehed\Security\Secret;
use RuntimeException;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\DoesNotPerformAssertions;
use PHPUnit\Framework\Attributes\Small;
use PHPUnit\Framework\TestCase;

/**
* @covers Firehed\JWT\JWT
*/
class JWTTest extends \PHPUnit\Framework\TestCase
#[CoversClass(JWT::class)]
#[Small]
class JWTTest extends TestCase
{

/**
* @dataProvider vectors
* @param array<string, mixed> $exp_claims
*/
#[DataProvider('vectors')]
public function testDecode(string $vector, array $exp_claims, KeyContainer $keys): void
{
$JWT = JWT::fromEncoded($vector, $keys);
Expand Down Expand Up @@ -70,9 +74,9 @@ public function testDecodeThrowsWithNonDictClaims(): void
}

/**
* @dataProvider vectors
* @param array<string, mixed> $claims
*/
#[DataProvider('vectors')]
public function testEncode(string $vector, array $claims, KeyContainer $keys): void
{
$tok = new JWT($claims);
Expand Down Expand Up @@ -203,9 +207,7 @@ public function testModifiedAlgorithmTriggersInvalidSignature(): void
$jwt->getClaims();
}

/**
* @doesNotPerformAssertions
*/
#[DoesNotPerformAssertions]
public function testConstruct(): void
{
$jwt = new JWT(['foo' => 'bar']);
Expand All @@ -224,7 +226,7 @@ public function testSetKeysReturnsthis(): void
/**
* @return array{string, array<string, mixed>, KeyContainer, bool}[]
*/
public function vectors(): array
public static function vectors(): array
{
// [
// encoded JWT,
Expand Down
11 changes: 7 additions & 4 deletions tests/KeyContainerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@

use Firehed\Security\Secret;

/**
* @covers Firehed\JWT\KeyContainer
*/
class KeyContainerTest extends \PHPUnit\Framework\TestCase
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\Small;
use PHPUnit\Framework\TestCase;

#[CoversClass(KeyContainer::class)]
#[Small]
class KeyContainerTest extends TestCase
{
public function testSetDefaultKeyReturnsThis(): void
{
Expand Down
21 changes: 9 additions & 12 deletions tests/SessionHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,25 @@
use Firehed\Security\Secret;
use InvalidArgumentException;
use OverflowException;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\Small;
use PHPUnit\Framework\TestCase;
use SessionHandlerInterface;

/**
* @covers Firehed\JWT\SessionHandler
*/
class SessionHandlerTest extends \PHPUnit\Framework\TestCase
#[CoversClass(SessionHandler::class)]
#[Small]
class SessionHandlerTest extends TestCase
{

/**
* Stores the data that would have gone to `setcookie`
* @var string
*/
private $cookieData = '';
private string $cookieData = '';

/** @var KeyContainer */
private $container;
private KeyContainer $container;

/** @var SessionHandlerInterface */
private $handler;
private SessionHandler $handler;

/**
*/
public function setUp(): void
{
$this->container = (new KeyContainer())
Expand Down

0 comments on commit ce3dcb3

Please sign in to comment.