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

'iat' added to payload shouldn't be automatic #19

Closed
brianjmiller opened this issue Jan 12, 2015 · 4 comments
Closed

'iat' added to payload shouldn't be automatic #19

brianjmiller opened this issue Jan 12, 2015 · 4 comments

Comments

@brianjmiller
Copy link
Contributor

I'm attempting to use this lib for handling signing of requests for an API where the API validates the payload against the received object assuring they match (at least in meaning) and the automatic addition of the 'iat' value means that the payload on the other side is not valid for the API in use. (If you are interested further see https://github.com/adlnet/xAPI-Spec/blob/master/xAPI.md#44-signed-statements).

I would think in general people relying on a security lib wouldn't want their data messed with in flight so I'd propose the automatic inclusion of 'iat' be disabled by default, but if that is too big of a breaking change for backwards compat I'd at least like to have a switch I can use on setPayload to prevent its automatic inclusion. (If it'd help I can submit a PR.)

Thanks for the work on the rest of the lib, it was easy to get going.

@odino
Copy link
Contributor

odino commented Jan 13, 2015

I think its good to have a flag that does skip this code altogether:

if (!isset($this->payload['iat'])) {
$now = new \DateTime('now');
$this->payload['iat'] = $now->format('U');
}

But at the same time id like to ask if the same would be achieved by using $payload['iat'] = false before setting the payload.

I honestly find this thing of generating the IAT automatically very useful, so that you dont really have to worry about generating things that JOSE can infer on its own. But again, Im open to changes since they might make sense for others!

Id love if you could put up a PR! :)

@brianjmiller
Copy link
Contributor Author

The problem here is the existence of the property in the payload on the other end of the signature so just setting it to false won't really help. Providing a way to turn it off would be fine with me, I certainly understand the usefulness in many (most?) situations. I'll submit a PR.

@Spomky
Copy link
Contributor

Spomky commented Feb 4, 2015

No claim or header should be set automaticaly.

The setPayload method should also set exactly what is requested. The $autoClaims parameter should be removed and iat claim should be explicitly set by developpers who want to use this claim.

Same goes for typ header defined in the constructor.

@odino
Copy link
Contributor

odino commented Feb 5, 2015

BTW Im happy to turn autoClaims by default off but at the same time there needs to be an option to have them on. Overall I think being pragmatic is nice, but be simpler and smart is much more important and if we can help people not re-doing the same things over and over (ie. creating a iat header everytime you instantiate a JWS).

It's like saying the Request object in symfony2 should be immutable. Well, definitely, but let's also be realistic :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants