Overall project health improvements

[giovannicimolin]

Hey folks,

There's a number of improvements and fixes we need to do to get this repository in a good shape again. For now, our focus is to get new builds out to Pypi, but that is (hopefully) about to get finished.

With that, we need to start working on a few things that will help the overall health of the project and maintainability moving forward.

Here are a few things I want to do with this repository before the end of the year:

• [ ] Add a changelog management tool (as suggested by @johnraz here: https://github.com/jazzband/django-rest-knox/pull/342#issuecomment-2097717682)
• [ ] Update developer documentation and readme
• [ ] Add more code samples to the example application and link it on the documentation
• [ ] Increase test coverage
• [ ] Simplify and document the development and release flow

Any other suggestions? Feel free to add them here! Do you want to pick up any items above? Create an issue and ping me there. :rocket:

[dgilmanAIDENTIFIED]

Thank you for noting that the most recent change will invalidate prior tokens. There was an earlier release where this wasn't done and caught a lot of people by surprise (see #266).

However, can you please consider upgrade paths that allow for some backwards compatible use of tokens? For our use of knox it is not acceptable to have everyone roll over all their tokens as part of a flag day. If knox can't offer some compatibility to allow for a smooth upgrade I will have to switch to a different project entirely!

Ongoing maintenance of this project via jazzband is welcome, however, compatibility is a must-have for a mature project like knox.

[johnraz]

@dgilmanAIDENTIFIED you have to balance maturity and maintainer capacity.

In the very case of the change of the salt removal and change of crypto package it was a conscious decision to simplify the maintainability and code simplicity. It also recused the amount of non native python dependencies.

I don’t think there was a better and possible way forward.

I would recommend implementing yourself a solution to encrypt current token in the 5.0+ way while still running pre 5.0 in a new colum and make sure to switch to that column once switching to 5.0+.

If you manage to do that in a generic way I really would recommend sharing that as an extension to knox for others to benefit from. It could even be added to the project.

cheers 👋

[dgilmanAIDENTIFIED]

Looking at the PR that moved you back to hashlib did this actually break broad backwards compatibility? If you never configured SECURE_HASH_ALGORITHM you'd still be on sha512 before and after and I don't see where that PR would actually change any outputs. (I would hope that both implementations of sha512 return the same thing!)

Maybe a better way to phrase the changelog is: if you configured knox to use a hashing algorithm only available in cryptography AND you no longer pull in cryptography as a dependency explicitly it will blow up? And in that case, the fix would just be to add cryptography by itself to your dependencies to keep using the old hasher?

[johnraz]

I believe this is the one that breaks backward compatibility -> https://github.com/jazzband/django-rest-knox/commit/c2ce297783f51c8e8c9404dde80005eb6b8dc916 and https://github.com/jazzband/django-rest-knox/issues/188

You can see how this was discussed … 5 years ago 😉

[johnraz]

Also @dgilmanAIDENTIFIED I wanted to emphasise that this sentence particularly triggers me:

Looking at the PR that moved you back to hashlib

There is no “you” but more of a “us”, there is a project backed by a community which is fluctuating through the years, and as a user of the library you are part of the community and as responsible of the project as any other user / contributor… I for one am not even using the project nor Django anymore and yet I still invest time here… I hope you understand my standpoint and how I feel about this.

[dgilmanAIDENTIFIED]

The salt removal PR was in 4.2.0 which is what inspired #266 .

[dgilmanAIDENTIFIED]

So if it wasn't the salt removal I think it's still an open question: did hashes change in 5.0? Are the only impacted users the ones who configured custom hash algorithsm?

[johnraz]

@davimagal good catch there didn’t realise salt removal was part of the previous release.

As for your question about compatibility, I couldn’t swear without testing, I guess you could easily test that out on a dummy project and switch from 4.2 to 5.0 and see if it breaks.

Letting others know would be really helpful as well 👍🏻

Also making a new issue to avoid highjacking this one further might be a good idea.

[giovannicimolin]

@johnraz Thanks for keeping the discussion going here! :heart:

@dgilmanAIDENTIFIED Welcome to our community!

I've just recently joined this project and moved it to Jazzband to ensure it would be able to receive upgrades, features and keep such a great little package in shape moving forward.

I can't say a thing for changes implemented in the past (since I wasn't even aware this repo existed), but we'll definitely consider backwards compatible upgrade paths when possible. This is a small library and each small amount of code added is a maintenance burden for everyone working on it.

For our use of knox it is not acceptable to have everyone roll over all their tokens as part of a flag day. If knox can't offer some compatibility to allow for a smooth upgrade I will have to switch to a different project entirely!

As mentioned by @johnraz, you can implement a solution for your use case and if it's generic enough contribute back to the project. As stated in our repo's licence: we don't offer warranties, but are open to all kinds of contributions (pull requests, reviews, issues, etc).

@dgilmanAIDENTIFIED Could you investigate if the tokens were really deprecated from version 4.2 to 5.0? It might haven been just miscommunication while writing the changelogs. Looking forward to see your findings on that one. :)

---

Since this issue discussion is off-topic to the original issue, I'll close it for now and reopen a different one for the repo improvements.