On Wed, May 10, 2017 at 10:51 AM, Vaishnavi Prabakaran
<vaishnaviprabaka...@gmail.com> wrote:
> Following recent removal of support to store password in plain text,
> modified the code to
> 1. Remove  "PASSWORD_TYPE_PLAINTEXT" macro
> 2. Instead of using "get_password_type" to retrieve the encryption method
> AND to check if the password is already encrypted or not, modified the code
> to
> a. Use "get_password_encryption_type" function to retrieve encryption
> method.
> b. Use "isPasswordEncrypted" function to check if the password is already
> encrypted or not.
> These changes are mainly to increase code readability and does not change
> underlying functionality.

Actually, this patch reduces readability.

> Attached the patch for community's review.

+ * Verify whether the given password is already encrypted or not
+ */
+isPasswordEncrypted(const char *password)
+   if(isMD5(password) || isSCRAM(password))
+       return true;
+   return false;
New hash functions may be added in the future, and we have now a
facility that can be easily extended for this purpose. We have been
already through a couple of commits to make that modular and I think
that it would be a bad idea to drop that. Please note that in this
facility we still need to be able to track passwords that are in a
plain format, because, even if Postgres does not store them in
cleartext, nothing prevents users to send to the server cleartext

In your patch, get_password_encryption_type() and
isPasswordEncrypted() are actually doing the same work. There is no
need for duplication as well.

In short, I am clearly -1 for this patch.

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to