Skip to content
This repository has been archived by the owner on Nov 17, 2020. It is now read-only.

Include password hashing function into definitions #117

Closed
michaelklishin opened this issue Jan 25, 2016 · 9 comments
Closed

Include password hashing function into definitions #117

michaelklishin opened this issue Jan 25, 2016 · 9 comments

Comments

@michaelklishin
Copy link
Member

…and use it during definitions import.

The idea was brought up in #116 but in a different context. It's not so much about those upgrading from 3.5.x — we have a documented procedure for them — but for those who will use a non-default function on 3.6.0+ and perform imports.

@michaelklishin
Copy link
Member Author

Note: if we can make upgrading via definitions import smooth for 3.5.x users, we definitely should.

@michaelklishin
Copy link
Member Author

Sorry, I'm not being specific enough: the definitions exported from 3.5.x will not have a hashing module field (unless we do another 3.5.x release, which is unlikely). So we need to be careful about what we do in that case:

  • Assume that we upgrade from 3.5.x and record hashing module as MD5
  • Assume that we upgrade from 3.6.0 and record hashing module as SHA-256

If there is no way to tell 3.5.x definitions from 3.6.0, we should do the latter and continue advising 3.5.x users to temporarily set hashing function to MD5 first.

@hairyhum let me know if the above isn't clear.

@michaelklishin
Copy link
Member Author

Someone suggests that if a hashing module value is missing, we should use the server default. Which makes total sense :)

@hairyhum
Copy link
Contributor

Someone suggests that if a hashing module value is missing, we should use the server default. Which makes total sense :)

So 3.5.x users will still have to set it to MD5 first?

@michaelklishin
Copy link
Member Author

Unless we can tell a JSON export file produced by 3.5.7 from the one produced by 3.6.0, yes.

@noahhaon
Copy link

This won't help existing users < 3.6.0, however it would be a great idea to add a version to your definition exports (and to go one further, HTTP headers in your API responses) such that special cases like this could be more easily handled in the future.

@michaelklishin
Copy link
Member Author

That's the point: we need to make sure the same issue doesn't pop up in the future.

@michaelklishin
Copy link
Member Author

@hairyhum's solution in #119 works well. Manual testing with 3.4.x and 3.5.x definitions was successful, 4 test suites pass.

@michaelklishin
Copy link
Member Author

Because definitions include RabbitMQ version we managed to cover versions from 3.0.0 through 3.5.7 and 3.6.0. Good job, @hairyhum.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants