frasertweedale / hs-jose

Haskell JOSE and JWT library
http://hackage.haskell.org/package/jose
Apache License 2.0
122 stars 46 forks source link

Caching getPOSIXTime calls #53

Closed begriffs closed 6 years ago

begriffs commented 6 years ago

Hi there, thanks for this excellent library. I'm switching PostgREST to use it in https://github.com/begriffs/postgrest/pull/919, but we noticed one regression. It used to be that we protected the getPOSIXTime function using the auto-update library. This way when the server gets high traffic it doesn't have to make this relatively expensive system call every time, but only one second apart at most.

-- ask for the OS time at most once per second      
getTime <- mkAutoUpdate defaultUpdateSettings {updateAction = getPOSIXTime}

Previously, with a different jwt library, we were passing this time into the jwt validation function inside our request handler like this:

time <- getTime
-- use time when checking jwt expiration

Is it possible to include this optimization inside hs-jose?

frasertweedale commented 6 years ago

@begriffs I'll provide version of the JWT validation functions that receive a UTCTime argument for the current time. Work for you?

begriffs commented 6 years ago

Yeah that would work fine, thanks!

frasertweedale commented 6 years ago

@begriffs could you please try out commit https://github.com/frasertweedale/hs-jose/commit/b3ce7f01ce260d34e01fcbd52fc05bcfa6243faa and verify that it is has the desired behaviour?

(Note: I only provided one new function, verifyClaimsAt; no explicit-time version of the validateClaimsSet is provided (and it shouldn't be needed in the common case either, because this validation is performed as part of verifyClaims.)

frasertweedale commented 6 years ago

@begriffs make that https://github.com/frasertweedale/hs-jose/commit/62d642bee388e93c0c2256e32fa63b33468187db (I just changed the order of some arguments).

steve-chavez commented 6 years ago

@frasertweedale I've tested the new verifyClaimsAt function in https://github.com/steve-chavez/postgrest/commit/14b5b3d7a8e4c38fb58136a821a81b577069629e and is working ok(all postgrest tests passing), would've great if you could do a release with the function to include the change in postgrest master branch.

frasertweedale commented 6 years ago

@begriffs @steve-chavez there ya go. I'll be cutting v0.7.0.0 within the next day or so.