mirror of
https://github.com/appwrite/appwrite.git
synced 2026-05-26 13:51:13 +00:00
Address stnguyen90 feedback: simplify JWT logic and fix test expectations
- Remove excessive logic and 1-minute expiration hack as requested - Simplify token expiry handling to directly use existing value - Remove problematic test that creates tokens with past expiry dates - Update existing test to properly verify JWT expiration behavior - Use proper base64url decoding for JWT payload testing
This commit is contained in:
@@ -62,25 +62,9 @@ class ResourceToken extends Model
|
||||
public function filter(Document $document): Document
|
||||
{
|
||||
$expire = $document->getAttribute('expire');
|
||||
$now = new \DateTime();
|
||||
|
||||
// Calculate expiration timestamp for JWT
|
||||
$expTimestamp = null;
|
||||
if ($expire !== null) {
|
||||
$expiryDate = new \DateTime($expire);
|
||||
$secondsUntilExpiry = $expiryDate->getTimestamp() - $now->getTimestamp();
|
||||
|
||||
// If token is expired, set expiration to 1 minute from now
|
||||
// We check for actual expiry later on route hooks for validation
|
||||
if ($secondsUntilExpiry <= 0) {
|
||||
$expTimestamp = $now->getTimestamp() + 60;
|
||||
} else {
|
||||
$expTimestamp = $expiryDate->getTimestamp();
|
||||
}
|
||||
}
|
||||
|
||||
// Use maxAge as fallback, but rely on exp in payload for actual expiration
|
||||
$jwt = new JWT(System::getEnv('_APP_OPENSSL_KEY_V1'), 'HS256', PHP_INT_MAX, 10);
|
||||
// Disable library auto-exp; rely solely on explicit exp in payload when set
|
||||
$jwt = new JWT(System::getEnv('_APP_OPENSSL_KEY_V1'), 'HS256', 0, 10);
|
||||
|
||||
$payload = [
|
||||
'tokenId' => $document->getId(),
|
||||
@@ -90,8 +74,9 @@ class ResourceToken extends Model
|
||||
];
|
||||
|
||||
// Set explicit expiration in JWT payload if we have an expiry date
|
||||
if ($expTimestamp !== null) {
|
||||
$payload['exp'] = $expTimestamp;
|
||||
if ($expire !== null) {
|
||||
$expiryDate = new \DateTime($expire);
|
||||
$payload['exp'] = $expiryDate->getTimestamp();
|
||||
}
|
||||
|
||||
$secret = $jwt->encode($payload);
|
||||
|
||||
@@ -94,6 +94,25 @@ class TokensConsoleClientTest extends Scope
|
||||
$dateValidator = new DatetimeValidator();
|
||||
$this->assertTrue($dateValidator->isValid($token['body']['expire']));
|
||||
|
||||
// Verify JWT contains correct expiration
|
||||
$this->assertNotEmpty($token['body']['secret']);
|
||||
$jwtParts = explode('.', $token['body']['secret']);
|
||||
$this->assertCount(3, $jwtParts, 'JWT should have 3 parts');
|
||||
|
||||
// Decode JWT payload (using base64url decoding)
|
||||
$payloadB64 = $jwtParts[1];
|
||||
// Convert base64url to base64
|
||||
$payloadB64 = str_replace(['-', '_'], ['+', '/'], $payloadB64);
|
||||
// Add padding if needed
|
||||
$payloadB64 .= str_repeat('=', (4 - strlen($payloadB64) % 4) % 4);
|
||||
$payload = json_decode(base64_decode($payloadB64), true);
|
||||
|
||||
$this->assertIsArray($payload, 'JWT payload should decode to an array');
|
||||
$this->assertArrayHasKey('exp', $payload, 'JWT payload should contain exp field');
|
||||
|
||||
$expectedExp = (new \DateTime($expiry))->getTimestamp();
|
||||
$this->assertEquals($expectedExp, $payload['exp'], 'JWT exp should match token expiry');
|
||||
|
||||
// Infinite expiry
|
||||
$token = $this->client->call(Client::METHOD_PATCH, '/tokens/' . $tokenId, array_merge([
|
||||
'content-type' => 'application/json',
|
||||
@@ -104,47 +123,20 @@ class TokensConsoleClientTest extends Scope
|
||||
|
||||
$this->assertEmpty($token['body']['expire']);
|
||||
|
||||
return $data;
|
||||
}
|
||||
|
||||
/**
|
||||
* @depends testCreateToken
|
||||
*/
|
||||
public function testExpiredTokenJWT(array $data): array
|
||||
{
|
||||
$fileId = $data['fileId'];
|
||||
$bucketId = $data['bucketId'];
|
||||
|
||||
// Create a token with an expiry date in the past (expired)
|
||||
$pastExpiry = DateTime::addSeconds(new \DateTime(), -3600); // 1 hour ago
|
||||
$expiredToken = $this->client->call(Client::METHOD_POST, '/tokens/buckets/' . $bucketId . '/files/' . $fileId, array_merge([
|
||||
'content-type' => 'application/json',
|
||||
'x-appwrite-project' => $this->getProject()['$id']
|
||||
], $this->getHeaders()), [
|
||||
'expire' => $pastExpiry,
|
||||
]);
|
||||
|
||||
$this->assertEquals(201, $expiredToken['headers']['status-code']);
|
||||
$this->assertEquals('files', $expiredToken['body']['resourceType']);
|
||||
|
||||
// Verify that the JWT is generated without causing a 500 error
|
||||
$this->assertNotEmpty($expiredToken['body']['secret']);
|
||||
|
||||
// Parse the JWT to verify expiration is set correctly for expired tokens
|
||||
$jwtParts = explode('.', $expiredToken['body']['secret']);
|
||||
$this->assertCount(3, $jwtParts, 'JWT should have 3 parts');
|
||||
|
||||
$payload = json_decode(base64_decode($jwtParts[1]), true);
|
||||
$this->assertArrayHasKey('exp', $payload, 'JWT payload should contain exp field');
|
||||
|
||||
// For expired tokens, exp should be set to a short time in the future (around 1 minute)
|
||||
$now = time();
|
||||
$this->assertGreaterThan($now, $payload['exp'], 'JWT exp should be in the future even for expired tokens');
|
||||
$this->assertLessThanOrEqual($now + 120, $payload['exp'], 'JWT exp should not be more than 2 minutes in the future for expired tokens');
|
||||
// Verify JWT does not contain exp for infinite expiry
|
||||
$jwtParts = explode('.', $token['body']['secret']);
|
||||
$payloadB64 = $jwtParts[1];
|
||||
$payloadB64 = str_replace(['-', '_'], ['+', '/'], $payloadB64);
|
||||
$payloadB64 .= str_repeat('=', (4 - strlen($payloadB64) % 4) % 4);
|
||||
$payload = json_decode(base64_decode($payloadB64), true);
|
||||
|
||||
$this->assertArrayNotHasKey('exp', $payload, 'JWT payload should not contain exp field for infinite expiry');
|
||||
|
||||
return $data;
|
||||
}
|
||||
|
||||
|
||||
|
||||
/**
|
||||
* @depends testCreateToken
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user