Mitigate CVE-2024-33663

[scy]

The library we're using for JWT-based authentication (i.e., the tokens the frontend is using to authenticate with the backend), python-jose, has an open security vulnerability, CVE-2024-33663, since April. There is no fix available, and development of the library seems to have stalled.

Having skimmed the details, I wonder whether we can mitigate this short-term by pinning the list of allowed algorithms. However, we might also just switch to a library that doesn't have this problem and that's actively maintained, e.g. PyJWT.

[jbethune]

I would like to fix this if you don't mind. IIRC I also implemented the original JWT support in dearmep.

Relevant docs: https://fastapi.tiangolo.com/tutorial/security/oauth2-jwt/

[scy]

Thanks for the offer. I'm still not super confident that it's a good time right now, because of the changes that might still be coming while I'm working on #269 and #286, but the JWT implementation is fairly isolated (basically just dearmep/api/authtoken.py), so I don't think that the chances for conflicts are super high.

Feel free to try! I assume you're planning on migrating to PyJWT instead of trying to pin the protocols with jose?

Also, I've noticed that there is no test coverage for that file. I'd appreciate some tests a lot.

[jbethune]

I assume you're planning on migrating to PyJWT instead of trying to pin the protocols with jose?

Yes. I finished the migration and I will push once I have added some tests. I need to figure out how we do tests in this project...

[scy]

Basically it's just standard pytest, but there are some wild things going on with some of the fixtures, in order to be able to modify the config on the fly or something, I don't even really remember. At some point we basically stopped writing tests altogether due to lack of time 😔 but we should start doing them again.

I guess you'd probably best start a new file like tests/test_tokens.py or something. I leave it up to you whether you're only testing the JWT functions themselves or do actual requests against the API. The latter is probably going to be significantly more complicated because you'd need to mock the SMS sending and stuff.

Maybe I don't leave it up to you then 😅 I'd recommend you do unit tests on just the token functions and not against the API. If you're eager to add some against the API, let's do a separate PR for those, to not endanger the upcoming release.

[jbethune]

I'll only test the tokens then. Edit: Using the fixtures wasn't that hard after all.