Re: Removal of plaintext password type references

On Wed, May 10, 2017 at 5:58 PM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:

> On 05/10/2017 08:01 AM, Michael Paquier wrote:
>
>> On Wed, May 10, 2017 at 10:51 AM, Vaishnavi Prabakaran
>> <vaishnaviprabakaran(at)gmail(dot)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.
>>
>
> Yeah, I don't think this is an improvement. Vaishnavi, did you find the
> current code difficult? Perhaps some extra comments somewhere would help?
>

Thanks for willing to add extra comments, And current code is not difficult
but kind of gives the impression that still plaintext password storage
exists by holding "PASSWORD_TYPE_PLAINTEXT" macro and having switch case
checks for this macro.

I see "get_password_type" function call is used for 2 purposes. One is to
check the actual password encryption type during authentication process and
another purpose is to verify if the password passed is encrypted or not -
used in create/alter role command before checking the strength of
password(via passwordcheck module). So, I thought my patch will make this
purpose clear. Nevertheless, if people thinks this actually reduces their
readability then I don't mind to go with the flow.

Thanks & Regards,
Vaishnavi
Fujitsu Australia.