Replace hs-jose with jose-jwt #3598
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wolfgangwalther
commented
Jun 16, 2024
wolfgangwalther
force-pushed
the
replace-jose
branch
2 times, most recently
from
a1c15b1 to
e5d0951
Compare
steve-chavez
approved these changes
Jun 16, 2024
wolfgangwalther
force-pushed
the
replace-jose
branch
2 times, most recently
from
7633239 to
d9aadb3
Compare
|
One thing that |
Timing dependent tests in the IO tests don't work too well when the next commit increases the JWT parsing performance. The remaining IO tests are for coverage and basic breakage. Loadtests are adapted so that performance regressions for JWT caching would be detected that way.
This removes one more dependency on Template Haskell.
wolfgangwalther
force-pushed
the
replace-jose
branch
from
d9aadb3 to
fef5197
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
hs-josedepends heavily on Template Haskell, butjose-jwtdoes not - one more step towards cross-compiling.The new
jose-jwtdoes not do more than the basic JWT verification for us - i.e. it checks the key and signature, but does not checkexp,aud,iatandnbfvalues. Thus, we have to do those ourself. I added some tests to confirm the previous behavior first, then made sure the refactor returns the same kind of error messages. Some of those could certainly be improved, because they have only beenshowinghs-jose's JWTError types. Not here, though.One "problem" remains: Two tests regarding the JWT cache were failing after this, because... parsing JWTs just became too fast. Thus the assumption
assert (first_dur - second_dur) > 0.3just didn't hold anymore. I tried rewriting those tests to use relative numbers, but now I have a different test failing:test_server_timing_jwt_should_not_decrease_when_caching_disabled. I could just revert the change to that test and call it a day... but I'd like to first discuss how much sense this test actually makes.Some test results from
postgrest-loadtest. Straight away onmainit has a throughput of 551 req/s for me. Of course, that doesn't say much, because the loadtest does not use any JWT anyway. So I added a very basic token, with empty payload, to all loadtests and then ran the loadtest again with and without cache on both main and this branch.Clearly JWT parsing performance has improved. Note that this is for the most minimal JWT you can assume, so no payload/claims at all. Performance for bigger JWT is probably dominated by performance of JSON parsing, not sure though.