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

Thanks, patch applied.

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

>
>
> On Mon, Jul 6, 2020 at 3:20 PM Aditya Toshniwal <
> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>
>>
>>
>> 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.
>>
> I have given the error message if the admin has only set the username and
> not the password though this message is not useful for normal users as this
> has been set by the admin.
>
> Thanks,
> Khushboo
>
>> 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"
>>
>

--
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*