Re: [pgAdmin4][Patch] - RM 5484 Support LDAP based auth also when users do not have the same DN structure

On Mon, Jul 6, 2020 at 3:16 PM Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

>
>
> On Mon, Jul 6, 2020 at 3:02 PM Aditya Toshniwal <
> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>
>>
>>
>> On Mon, Jul 6, 2020 at 2:55 PM Khushboo Vashi <
>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Aditya,
>>>
>>> Please find the attached updated patch.
>>>
>>> On Mon, Jul 6, 2020 at 11:44 AM Aditya Toshniwal <
>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Khushboo,
>>>>
>>>> I suggest,
>>>> 1) Remove the commented code
>>>>
>>>> + # if 'mail' in entry:
>>>>
>>>> + # user_email = entry['mail'].value
>>>>
>>> Removed.
>>>
>>>> 2) Change the below condition to check "is not None" instead which
>>>> makes more sense.
>>>>
>>>> + if not self.bind_user or not self.bind_pass:
>>>>
>>>> "is not None" will not check empty strings. Though the default value is
>>> None but the user might set an empty string into config_local.py file.
>>>
>> A password can be empty. And setting this config var indicates the user
>> wants to set it as its an optional config.
>>
> Yes, it's an optional config but if the user has set BIND_USER param then
> password is mandatory.
>
I agree that password is mandatory. But, one can have a blank password.
Which also suggests maybe there should be some meaningful error thrown to
the user if the user sets the username but not the password.

>
>>> Thanks,
>>> Khushboo
>>>
>>>> Other changes looks fine.
>>>>
>>>> On Mon, Jul 6, 2020 at 11:29 AM Akshay Joshi <
>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Aditya,
>>>>>
>>>>> Can you please review it.
>>>>>
>>>>> On Mon, Jul 6, 2020 at 11:17 AM Khushboo Vashi <
>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Please find the attached patch for the RM 5484 - Support LDAP based
>>>>>> auth also when users do not have the same DN structure.
>>>>>>
>>>>>> Currently, pgAdmin only supports LDAP authentication with the same DN
>>>>>> structure. With this patch, the LDAP authentication will also support the
>>>>>> different DN by setting the dedicated user for the LDAP connection in the
>>>>>> config file.
>>>>>>
>>>>>> Thanks,
>>>>>> Khushboo
>>>>>>
>>>>>>
>>>>>
>>>>> --
>>>>> *Thanks & Regards*
>>>>> *Akshay Joshi*
>>>>>
>>>>> *Sr. Software Architect*
>>>>> *EnterpriseDB Software India Private Limited*
>>>>> *Mobile: +91 976-788-8246*
>>>>>
>>>>
>>>>
>>>> --
>>>> Thanks and Regards,
>>>> Aditya Toshniwal
>>>> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
>>>> "Don't Complain about Heat, Plant a TREE"
>>>>
>>>
>>
>> --
>> Thanks and Regards,
>> Aditya Toshniwal
>> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
>> "Don't Complain about Heat, Plant a TREE"
>>
>

--
Thanks and Regards,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"