Deprecate add_hash and check_pass functions

[riverrun]

Problem

In version 4, the add_hash and check_pass functions were added. These functions were intended to be useful convenience functions that would reduce boilerplate code.

Unfortunately, it seems that several developers, especially new users of Comeonin, find these functions (and the resulting api) confusing. In addition, these functions do not seem to be widely used in authentication libraries, and, on reflection, they are probably not as useful as I had hoped.

Solution

I propose to deprecate these two functions in version 5.4, with the intention of removing them in version 6.0 (which will be in 2-3 months' time).

I hope that this will simplify the api and make comeonin, and the related password hashing libraries, easier to use.

Final note

This decision to remove these functions is not final. It also depends on the feedback I get in the next 2-3 months.

[josevalim]

I agree. :) We don't those in mix phx.gen.auth either!

[riverrun]

Deprecation notices (documentation) added to version 5.4.0

[JonRowe]

👋 What is the intended replacement? The deprecations do not mention what you should swap them out for? (As an aside I found this rather confusing because the warning comes from Bcrypt but there is no mention of this in their changelog, so prehaps amending the message to at least reference it is a comeonin deprecation)

[josevalim]

@JonRowe the easiest is to look at their current implementations. add_hash in particular is quite straight-forward: https://github.com/riverrun/comeonin/blob/master/lib/comeonin.ex#L32

[JonRowe]

When I eventually found the source of the deprecation I did just vendor the logic from check_pass, but this was a convient one liner it would be nice if something replaced the deprecated functions :)

[datafoo]

Coming late to the party but for me check_pass is useful and I do not understand why it was deprecated.

If there was still confusion related to riverrun/argon2_elixir#38, then improving documentation would probably have been better than simply removing useful functions.

Also, why is this issue still open considering the deprecation is effective.

[josevalim]

👍 for closing this issue.