Closed egeersoz closed 4 years ago
Thanks for raising the issue.
Some of the problems with organization is that argon2_elixir use
-s functions defined in Comeonin, but the function documentation cannot be accessed directly via argon2_elixir, and so I put the documentation for those functions in the module documentation.
One other problem is that these libraries are very widely used, and it is difficult to change the names of functions without causing confusion / problems for users when they update the library.
Anyway, I will have a further look at this issue in the new year and get back to you then.
One more thing: for most apps, you will just need to use the add_hash
and check_pass
functions, which are convenience functions. Three are examples of how to use them in the module docs, and I will look at making it clearer that those are the only functions that most users will need to use.
Updated the documentation in 2.2.0 - the hex docs can be seen here.
If you have any comments, queries or suggestions, please let me know.
Closing this issue for now.
First of all, thank you for your work. I'm really glad this exists as an alternative to Bcrypt.
I've been trying to wrap my mind around the various functions and behaviors implemented by this library, and I'm having a bit of trouble. Since authentication logic is important and misunderstandings can potentially result in severe security flaws, I want to make sure I understand everything thoroughly, and that other people do, as well.
To start with an example: what is the difference between
check_pass
andverify_pass
? The function names suggest they are identical, but based on the examples they are different. Is there a reason both of these exist, when they seem to do the same thing (albeit with different param types)? When is a good time to use one or the other? I looked at the source code -- Comeonin'scheck_pass
callsverify_pass
internally, and that returns a true or false. But Argon2'sverify_pass
can additionally raise an ArgumentError...Similarly, there seems to be a lot of redundancy between
add_hash
andhash_pwd_salt
. Again, I understand thatadd_hash
returns a map that is meant to be used in an Ecto changeset, and it works fine for that purpose. But the way these functions are named makes the reader wonder whetheradd_hash
uses a salt as well (I had to read the source code to verify, just for ease of mind). Maybe rename it toadd_hash_salt
, or even better,add_hash_salt_to_changeset
, since that's what it does? (I also wish there was further standardization in function names - some functions refer to passwords as "pass" and others refer to them as "pwd" - just a thought.)Generally, the organization of the documentation on Hex.pm is a bit confusing, and seems non-standard. Information about various functions is scattered under 'Options' and 'Examples' headers, and the 'Functions' header contains only a single function,
gen_salt
. This makes it challenging to understand the proper and recommended usage of each function. For example, I was trying to find the possible returns ofcheck_pass
, and after a lot of browsing around I ended up having to read the source code of Comeonin itself, despite the fact that the function is called from Argon2. I get that it's a behavior, but properly organized documentation that clearly shows param and return types would eliminate the need to read the source code. Adding hyperlinks to the relevant sections of the Comeonin docs would also help. :)I know that the Comeonin framework has gone through multiple iterations and redesigns, so maybe things are still in flux. Once I fully grok this library and how to properly use it in a "real-life" project, I'll submit a tutorial with a reference project link so that it can be added to the docs. I just wanted to submit this piece of feedback so that you're aware of the difficulties users might be facing. Thank you.