Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #191: Allow leeway to handle clock skew #248

Merged
merged 1 commit into from
Aug 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 43 additions & 13 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,13 @@ Just use the builder to create a new JWT/JWS tokens:
```php
use Lcobucci\JWT\Builder;

$time = time();
$token = (new Builder())->issuedBy('http://example.com') // Configures the issuer (iss claim)
->canOnlyBeUsedBy('http://example.org') // Configures the audience (aud claim)
->identifiedBy('4f1g23a12aa', true) // Configures the id (jti claim), replicating as a header item
->issuedAt(time()) // Configures the time that the token was issue (iat claim)
->canOnlyBeUsedAfter(time() + 60) // Configures the time that the token can be used (nbf claim)
->expiresAt(time() + 3600) // Configures the expiration time of the token (exp claim)
->issuedAt($time) // Configures the time that the token was issue (iat claim)
->canOnlyBeUsedAfter($time + 60) // Configures the time that the token can be used (nbf claim)
->expiresAt($time + 3600) // Configures the expiration time of the token (exp claim)
->with('uid', 1) // Configures a new claim, called "uid"
->getToken(); // Retrieves the generated token

Expand Down Expand Up @@ -69,7 +70,7 @@ echo $token->getClaim('uid'); // will print "1"

### Validating

We can easily validate if the token is valid (using the previous token as example):
We can easily validate if the token is valid (using the previous token and time as example):

```php
use Lcobucci\JWT\ValidationData;
Expand All @@ -79,15 +80,37 @@ $data->setIssuer('http://example.com');
$data->setAudience('http://example.org');
$data->setId('4f1g23a12aa');

var_dump($token->validate($data)); // false, because token cannot be used before of now() + 60
var_dump($token->validate($data)); // false, because token cannot be used before now() + 60

$data->setCurrentTime(time() + 61); // changing the validation time to future
$data->setCurrentTime($time + 61); // changing the validation time to future

var_dump($token->validate($data)); // true, because current time is between "nbf" and "exp" claims

$data->setCurrentTime(time() + 4000); // changing the validation time to future
$data->setCurrentTime($time + 4000); // changing the validation time to future

var_dump($token->validate($data)); // false, because token is expired since current time is greater than exp

// We can also use the $leeway parameter to deal with clock skew (see notes below)
// If token's claimed time is invalid but the difference between that and the validation time is less than $leeway,
// then token is still considered valid
$dataWithLeeway = new ValidationData($time, 20);
$dataWithLeeway->setIssuer('http://example.com');
$dataWithLeeway->setAudience('http://example.org');
$dataWithLeeway->setId('4f1g23a12aa');

var_dump($token->validate($dataWithLeeway)); // false, because token can't be used before now() + 60, not within leeway

$dataWithLeeway->setCurrentTime($time + 51); // changing the validation time to future

var_dump($token->validate($dataWithLeeway)); // true, because current time plus leeway is between "nbf" and "exp" claims

$dataWithLeeway->setCurrentTime($time + 3610); // changing the validation time to future but within leeway

var_dump($token->validate($dataWithLeeway)); // true, because current time - 20 seconds leeway is less than exp

$dataWithLeeway->setCurrentTime($time + 4000); // changing the validation time to future outside of leeway

var_dump($token->validate($dataWithLeeway)); // false, because token is expired since current time is greater than exp
```

#### Important
Expand All @@ -97,6 +120,11 @@ var_dump($token->validate($data)); // false, because token is expired since curr
configured in ```ValidationData``` they will be ignored by ```Token::validate()```.
- ```exp```, ```nbf``` and ```iat``` claims are configured by default in ```ValidationData::__construct()```
with the current UNIX time (```time()```).
- The optional ```$leeway``` parameter of ```ValidationData``` will cause us to use that number of seconds of leeway
when validating the time-based claims, pretending we are further in the future for the "Issued At" (```iat```) and "Not
Before" (```nbf```) claims and pretending we are further in the past for the "Expiration Time" (```exp```) claim. This
allows for situations where the clock of the issuing server has a different time than the clock of the verifying server,
as mentioned in [section 4.1 of RFC 7519](https://tools.ietf.org/html/rfc7519#section-4.1).

## Token signature

Expand All @@ -119,13 +147,14 @@ use Lcobucci\JWT\Builder;
use Lcobucci\JWT\Signer\Hmac\Sha256;

$signer = new Sha256();
$time = time();

$token = (new Builder())->issuedBy('http://example.com') // Configures the issuer (iss claim)
->canOnlyBeUsedBy('http://example.org') // Configures the audience (aud claim)
->identifiedBy('4f1g23a12aa', true) // Configures the id (jti claim), replicating as a header item
->issuedAt(time()) // Configures the time that the token was issue (iat claim)
->canOnlyBeUsedAfter(time() + 60) // Configures the time that the token can be used (nbf claim)
->expiresAt(time() + 3600) // Configures the expiration time of the token (exp claim)
->issuedAt($time) // Configures the time that the token was issue (iat claim)
->canOnlyBeUsedAfter($time + 60) // Configures the time that the token can be used (nbf claim)
->expiresAt($time + 3600) // Configures the expiration time of the token (exp claim)
->with('uid', 1) // Configures a new claim, called "uid"
->sign($signer, 'testing') // creates a signature using "testing" as key
->getToken(); // Retrieves the generated token
Expand All @@ -146,13 +175,14 @@ use Lcobucci\JWT\Signer\Rsa\Sha256; // you can use Lcobucci\JWT\Signer\Ecdsa\Sha

$signer = new Sha256();
$privateKey = new Key('file://{path to your private key}');
$time = time();

$token = (new Builder())->issuedBy('http://example.com') // Configures the issuer (iss claim)
->canOnlyBeUsedBy('http://example.org') // Configures the audience (aud claim)
->identifiedBy('4f1g23a12aa', true) // Configures the id (jti claim), replicating as a header item
->issuedAt(time()) // Configures the time that the token was issue (iat claim)
->canOnlyBeUsedAfter(time() + 60) // Configures the time that the token can be used (nbf claim)
->expiresAt(time() + 3600) // Configures the expiration time of the token (exp claim)
->issuedAt($time) // Configures the time that the token was issue (iat claim)
->canOnlyBeUsedAfter($time + 60) // Configures the time that the token can be used (nbf claim)
->expiresAt($time + 3600) // Configures the expiration time of the token (exp claim)
->with('uid', 1) // Configures a new claim, called "uid"
->sign($signer, $privateKey) // creates a signature using your private key
->getToken(); // Retrieves the generated token
Expand Down
2 changes: 1 addition & 1 deletion src/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ public function setSubject($subject, $replicateAsHeader = false)
}

/**
* Configures a registed claim
* Configures a registered claim
*
* @param string $name
* @param mixed $value
Expand Down
27 changes: 18 additions & 9 deletions src/ValidationData.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,24 +24,31 @@ class ValidationData
*/
private $items;

/**
* The leeway (in seconds) to use when validating time claims
* @var int
*/
private $leeway;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering if it makes sense to have this as a property... I mean, it's only used for configuring the current time, maybe it would be better to simply pass it to ValidationData#setCurrentTime()... @Ocramius any thoughts?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm completely fine with doing it either way. Leaving it as it is makes sense if it's fairly common to use the setCurrentTime() method 1 or more times after initial construction of the object while keeping the same leeway. However, if users would want to keep changing the leeway without constructing a new object, or if they rarely use setCurrentTime() directly at all, then passing leeway as a parameter to setCurrentTime() makes more sense.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@m777z I'm have no clue on how users are using this object, and since it will be deprecated very soon I don't think we should debate this much. Let's go with your suggested approach and if needed we change it. Thanks!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lcobucci I am assuming you mean it is okay to just leave things as they are for this part. If so, are there other changes I need to make before this pull request is merged?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@m777z that's correct. There're some small CS changes (which I'll handle ASAP), I just didn't manage to work on this. Plan to work on it tomorrow


/**
* Initializes the object
*
* @param int $currentTime
* @param int $leeway
*/
public function __construct($currentTime = null)
public function __construct($currentTime = null, $leeway = 0)
{
$currentTime = $currentTime ?: time();
$currentTime = $currentTime ?: time();
$this->leeway = (int) $leeway;

$this->items = [
'jti' => null,
'iss' => null,
'aud' => null,
'sub' => null,
'iat' => $currentTime,
'nbf' => $currentTime,
'exp' => $currentTime
'sub' => null
];

$this->setCurrentTime($currentTime);
}

/**
Expand Down Expand Up @@ -91,9 +98,11 @@ public function setSubject($subject)
*/
public function setCurrentTime($currentTime)
{
$this->items['iat'] = (int) $currentTime;
$this->items['nbf'] = (int) $currentTime;
$this->items['exp'] = (int) $currentTime;
$currentTime = (int) $currentTime;

$this->items['iat'] = $currentTime + $this->leeway;
$this->items['nbf'] = $currentTime + $this->leeway;
$this->items['exp'] = $currentTime - $this->leeway;
}

/**
Expand Down
24 changes: 24 additions & 0 deletions test/functional/UnsignedTokenTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,30 @@ public function tokenValidationShouldReturnFalseWhenExpectedDataDontMatch(Valida
$this->assertFalse($generated->validate($data));
}

/**
* @test
*
* @depends builderCanGenerateAToken
*
* @covers Lcobucci\JWT\Builder
* @covers Lcobucci\JWT\Parser
* @covers Lcobucci\JWT\Token
* @covers Lcobucci\JWT\ValidationData
* @covers Lcobucci\JWT\Claim\Factory
* @covers Lcobucci\JWT\Claim\Basic
* @covers Lcobucci\JWT\Claim\EqualsTo
* @covers Lcobucci\JWT\Claim\GreaterOrEqualsTo
* @covers Lcobucci\JWT\Parsing\Encoder
* @covers Lcobucci\JWT\Parsing\Decoder
*/
public function tokenValidationShouldReturnTrueWhenExpectedDataMatchBecauseOfLeeway(Token $generated)
{
$notExpiredDueToLeeway = new ValidationData(self::CURRENT_TIME + 3020, 50);
$notExpiredDueToLeeway->setAudience('http://client.abc.com');
$notExpiredDueToLeeway->setIssuer('http://api.abc.com');
$this->assertTrue($generated->validate($notExpiredDueToLeeway));
}

public function invalidValidationData()
{
$expired = new ValidationData(self::CURRENT_TIME + 3020);
Expand Down
69 changes: 69 additions & 0 deletions test/unit/TokenTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,40 @@ public function validateShouldReturnFalseWhenThereIsAtLeastOneFailedValidatableC
$this->assertFalse($token->validate($data));
}

/**
* @test
*
* @uses Lcobucci\JWT\Token::__construct
* @uses Lcobucci\JWT\ValidationData
* @uses Lcobucci\JWT\Claim\Basic
* @uses Lcobucci\JWT\Claim\EqualsTo
* @uses Lcobucci\JWT\Claim\LesserOrEqualsTo
* @uses Lcobucci\JWT\Claim\GreaterOrEqualsTo
*
* @covers Lcobucci\JWT\Token::validate
* @covers Lcobucci\JWT\Token::getValidatableClaims
*/
public function validateShouldReturnFalseWhenATimeBasedClaimFails()
{
$now = time();

$token = new Token(
[],
[
'iss' => new EqualsTo('iss', 'test'),
'iat' => new LesserOrEqualsTo('iat', $now),
'nbf' => new LesserOrEqualsTo('nbf', $now + 20),
'exp' => new GreaterOrEqualsTo('exp', $now + 500),
'testing' => new Basic('testing', 'test')
]
);

$data = new ValidationData($now + 10);
$data->setIssuer('test');

$this->assertFalse($token->validate($data));
}

/**
* @test
*
Expand All @@ -380,6 +414,7 @@ public function validateShouldReturnFalseWhenThereIsAtLeastOneFailedValidatableC
public function validateShouldReturnTrueWhenThereAreNoFailedValidatableClaims()
{
$now = time();

$token = new Token(
[],
[
Expand All @@ -396,6 +431,40 @@ public function validateShouldReturnTrueWhenThereAreNoFailedValidatableClaims()
$this->assertTrue($token->validate($data));
}

/**
* @test
*
* @uses Lcobucci\JWT\Token::__construct
* @uses Lcobucci\JWT\ValidationData
* @uses Lcobucci\JWT\Claim\Basic
* @uses Lcobucci\JWT\Claim\EqualsTo
* @uses Lcobucci\JWT\Claim\LesserOrEqualsTo
* @uses Lcobucci\JWT\Claim\GreaterOrEqualsTo
*
* @covers Lcobucci\JWT\Token::validate
* @covers Lcobucci\JWT\Token::getValidatableClaims
*/
public function validateShouldReturnTrueWhenLeewayMakesAllTimeBasedClaimsTrueAndOtherClaimsAreTrue()
{
$now = time();

$token = new Token(
[],
[
'iss' => new EqualsTo('iss', 'test'),
'iat' => new LesserOrEqualsTo('iat', $now),
'nbf' => new LesserOrEqualsTo('nbf', $now + 20),
'exp' => new GreaterOrEqualsTo('exp', $now + 500),
'testing' => new Basic('testing', 'test')
]
);

$data = new ValidationData($now + 10, 20);
$data->setIssuer('test');

$this->assertTrue($token->validate($data));
}

/**
* @test
*
Expand Down
51 changes: 42 additions & 9 deletions test/unit/ValidationDataTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,19 @@ public function constructorShouldConfigureTheItems()
$this->assertAttributeSame($expected, 'items', $data);
}

/**
* @test
*
* @covers Lcobucci\JWT\ValidationData::__construct
*/
public function constructorWithLeewayShouldConfigureTheItems()
{
$expected = $this->createExpectedData(null, null, null, null, 111, 111, 89);
$data = new ValidationData(100, 11);

$this->assertAttributeSame($expected, 'items', $data);
}

/**
* @test
*
Expand Down Expand Up @@ -114,6 +127,22 @@ public function setCurrentTimeShouldChangeTheTimeBasedValues()
$this->assertAttributeSame($expected, 'items', $data);
}

/**
* @test
*
* @uses Lcobucci\JWT\ValidationData::__construct
*
* @covers Lcobucci\JWT\ValidationData::setCurrentTime
*/
public function setCurrentTimeShouldChangeTheTimeBasedValuesUsingLeeway()
{
$expected = $this->createExpectedData(null, null, null, null, 30, 30, 10);
$data = new ValidationData(15, 10);
$data->setCurrentTime(20);

$this->assertAttributeSame($expected, 'items', $data);
}

/**
* @test
*
Expand Down Expand Up @@ -196,11 +225,13 @@ public function claimValues()
}

/**
* @param string $id
* @param string $sub
* @param string $iss
* @param string $aud
* @param int $time
* @param string|null $id
* @param string|null $sub
* @param string|null $iss
* @param string|null $aud
* @param int $iat
* @param int|null $nbf
* @param int|null $exp
*
* @return array
*/
Expand All @@ -209,16 +240,18 @@ private function createExpectedData(
$sub = null,
$iss = null,
$aud = null,
$time = 1
$iat = 1,
$nbf = null,
$exp = null
) {
return [
'jti' => $id !== null ? (string) $id : null,
'iss' => $iss !== null ? (string) $iss : null,
'aud' => $aud !== null ? (string) $aud : null,
'sub' => $sub !== null ? (string) $sub : null,
'iat' => $time,
'nbf' => $time,
'exp' => $time
'iat' => $iat,
'nbf' => $nbf !== null ? $nbf: $iat,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's sad that we can't use ?? to simplify this due to PHP 5.x compatibility 😭

'exp' => $exp !== null ? $exp: $iat
];
}
}