-
Notifications
You must be signed in to change notification settings - Fork 133
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
setPayload flag and customized header properties #21
Changes from all commits
4278a06
c2e29fd
e99ec80
fbcdf92
3186960
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,9 +34,9 @@ class JWT | |
*/ | ||
public function __construct(array $payload, array $header) | ||
{ | ||
$this->payload = $payload; | ||
$this->header = $header; | ||
$this->encoder = new Base64UrlSafeEncoder(); | ||
$this->setPayload($payload); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about assigning directly as before? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is an artifact of the intermediary step where I had added a flag that you passed to turn off automatic claims inclusion which got backed out when switching to the subclass approach suggested by @odino. (That intermediary step has since been squashed+rebased out.) I can back this one out, though some would probably argue it is a good idea to use the public interface to prevent duplicated efforts if these setters ever do something more meaningful. Let me know and I'll fix it up. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @brianjmiller if there is no special logic in setters and/or no need to expose them externally I would remove them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was wrong, this is to make sure that the method of the subclass is called which does have special logic. For instance in EasyJWS |
||
$this->setHeader($header); | ||
$this->setEncoder(new Base64UrlSafeEncoder()); | ||
} | ||
|
||
/** | ||
|
@@ -81,11 +81,6 @@ public function setPayload(array $payload) | |
{ | ||
$this->payload = $payload; | ||
|
||
if (!isset($this->payload['iat'])) { | ||
$now = new \DateTime('now'); | ||
$this->payload['iat'] = $now->format('U'); | ||
} | ||
|
||
return $this; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
<?php | ||
|
||
namespace Namshi\JOSE; | ||
|
||
/** | ||
* Class providing an easy to use JWS implementation. | ||
*/ | ||
class SimpleJWS extends JWS | ||
{ | ||
/** | ||
* Constructor | ||
* | ||
* @param array $header An associative array of headers. The value can be any type accepted by json_encode or a JSON serializable object | ||
* @see http://php.net/manual/en/function.json-encode.php | ||
* @see http://php.net/manual/en/jsonserializable.jsonserialize.php | ||
* @see https://tools.ietf.org/html/draft-ietf-jose-json-web-signature-41#section-4 | ||
* } | ||
*/ | ||
public function __construct($header = array()) | ||
{ | ||
if (!isset($header['typ'])) { | ||
$header['typ'] = 'JWS'; | ||
} | ||
parent::__construct($header); | ||
} | ||
|
||
/** | ||
* Sets the payload of the current JWS with an issued at value in the 'iat' property. | ||
* | ||
* @param array $payload | ||
*/ | ||
public function setPayload(array $payload) | ||
{ | ||
if (!isset($payload['iat'])) { | ||
$now = new \DateTime('now'); | ||
$payload['iat'] = $now->format('U'); | ||
} | ||
|
||
return parent::setPayload($payload); | ||
} | ||
|
||
/** | ||
* Checks that the JWS has been signed with a valid private key by verifying it with a public $key | ||
* and the token is not expired. | ||
* | ||
* @param resource|string $key | ||
* @param string $algo The algorithms this JWS should be signed with. Use it if you want to restrict which algorithms you want to allow to be validated. | ||
* | ||
* @return bool | ||
*/ | ||
public function isValid($key, $algo = null) | ||
{ | ||
return $this->verify($key, $algo) && ! $this->isExpired(); | ||
} | ||
|
||
/** | ||
* Checks whether the token is expired based on the 'exp' value. | ||
* | ||
* @return bool | ||
*/ | ||
protected function isExpired() | ||
{ | ||
$payload = $this->getPayload(); | ||
|
||
if (isset($payload['exp']) && is_numeric($payload['exp'])) { | ||
$now = new \DateTime('now'); | ||
|
||
return ($now->format('U') - $payload['exp']) > 0; | ||
} | ||
|
||
return false; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH I think this will be confusing, as the algorith can either be the algorithm itself or a dictionary or headers. I think we should change the constructor rather than have this as it leads to misunderstanding and legacy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm okay with that, I was just trying to maintain backwards compatibility to prevent the big version bump.