docs(changelog): bump to 5.0.0, add token warning

[max-wittig]

I've increased the version to major, as this kind of a big change asks for one.

Fixes #357

/cc @johnraz

[johnraz]

Thanks !

[jmsmkn]

Bit confused by these warnings now. We upgraded to 4.2 a year ago, which broke existing tokens. Will upgrading to 5.0.0 break them again? Or was this warning just added twice to highlight it?

[giovannicimolin]

@jmsmkn I'm not sure, and I haven't had the time to test if they will actually break yet.
It might have been a miscommunication and the warning isn't real, but I'm not sure.

If you have some time could you test this and report back here?

[jmsmkn]

It does indeed break them, caused by #272, bit of a pain considering the default is not to use that feature.

[jmsmkn]

Looking through that PR:

• Why was sha added? Seems unnecessary.
• Why was max_length increased to include CONSTANTS.MAXIMUM_TOKEN_PREFIX_LENGTH? CONSTANTS.MAXIMUM_TOKEN_PREFIX_LENGTH is not considered when saving the token. The TOKEN_KEY_LENGTH was increased in that same PR, which causes the breakage of existing tokens. Can't old-style, shorter token keys be handled when authenticating and then updated to the new longer style?

[max-wittig]

Hi @jmsmkn @giovannicimolin This MR (#272) does not break it. I added tests at the time to ensure that, however the release does.

What breaks the token compatibility is: b02a155

I filed an issue here regarding this as well: #357

[jmsmkn]

b02a155 does not break anything as that is just adding a warning to the documentation.

I tested this by generating the tokens with 4.2.0 (which already includes the changes of the salt removal described by b02a155), then installed 5.0.1, migrated, and ran a request against that process.

Both 4.1.0 -> 4.2.0 and 4.2.0 -> 5.0.1 break the tokens.

[johnraz]

@jmsmkn Thanks for taking the time to test this 👌🏻

@max-wittig I reviewed the PR and the test there is not testing that a token generated before your change will work after. In order to properly do that you would have to checkout the code at the previous version then at the next etc. So we cannot state strongly that this is not breaking it.

I do trust @jmsmkn carefully tested this manually and I believe there is an actual different breakage between 4.2.0 and 5.0.0.

Now, @jmsmkn if you feel things should be done differently, I think it would be more constructive to open a PR so we can discuss the code and your change proposal rather than hypothetically talking about what could / should have been done.

5.0.1 is out, there is no going back so let’s look ahead and try to avoid breaking changes in the future.

Cheers!

[max-wittig]

@max-wittig I reviewed the PR and the test there is not testing that a token generated before your change will work after. In order to properly do that you would have to checkout the code at the previous version then at the next etc. So we cannot state strongly that this is not breaking it.

Ahh I see. It just tested that it works when changing the prefix. I wasn't aware of the breakage that I've caused! I'm sorry!

5.0.1 is out, there is no going back so let’s look ahead and try to avoid breaking changes in the future.

Yes that is true. Maybe there needs to be a test in CI using tokens in main and comparing them with any additional PR to see if the tokens still work.

[jmsmkn]

Yes that is true. Maybe there needs to be a test in CI using tokens in main and comparing them with any additional PR to see if the tokens still work.

Added in #362 for tokens from 4.2.0 without prefixes.