Time units mismatch

[luisgoncalves]

The ShouldFailWithTimestampInThePast test fails if a timestamp 100 seconds behind is used. The tests are using 1 day which contains more than 1000 seconds, and that's way they succeed.

The ConvertToUnixTimestamp method returns the number of seconds since the epoc. However, the GetBewit method (and probably others) then use var expiration = Math.Floor(now / 1000) + ttlSec;. The value of now is already in seconds.

Since the timestamp in the bewit is in seconds, I'd suggest always using seconds as the time unit. This would also remove all the /1000 and *1000 code.

[luisgoncalves]

Another option is to just change the ConvertToUnixTimestamp method to return TotalMilliseconds but it would keep all the aritmetic operations.

[technicaljoe]

I seems to have encountered the same issue, but only when using dflydev-hawk to talk to hawknet, but not from hawknet to hawknet. Just thought I'd ask in case this is indeed a confirmed issue.

[luisgoncalves]

hawknet makes is coherent (despite wrong) when generating and verifying the bewit. That's why you may not notice the behavior when using only hawknet.

[technicaljoe]

Thanks for the explanation! Would it be sensible for me to create a fork version with the fix and base our solution off that? Or have you encounter other solutions?

[luisgoncalves]

I made the fix locally because I had to make this work quickly. I think you could do it and submit a pull request to pcibraro.

[pcibraro]

I am very sorry guys. I have been out for the last few days and I somehow missed this thread. Luis, feel free to send me a pull request with the fix and I will push it in the repository as soon as possible. Thanks

[technicaljoe]

Thanks @luisgoncalves and @pcibraro!

[technicaljoe]

Would this latest change be pushed into NuGet?

[pcibraro]

It has been pushed today. Version 1.3.1.0. Thanks