Thanks, patch applied. On Mon, Jul 6, 2020 at 4:05 PM Khushboo Vashi < khushboo.va...@enterprisedb.com> wrote:
> > > On Mon, Jul 6, 2020 at 3:20 PM Aditya Toshniwal < > aditya.toshni...@enterprisedb.com> wrote: > >> >> >> On Mon, Jul 6, 2020 at 3:16 PM Khushboo Vashi < >> khushboo.va...@enterprisedb.com> wrote: >> >>> >>> >>> On Mon, Jul 6, 2020 at 3:02 PM Aditya Toshniwal < >>> aditya.toshni...@enterprisedb.com> wrote: >>> >>>> >>>> >>>> On Mon, Jul 6, 2020 at 2:55 PM Khushboo Vashi < >>>> khushboo.va...@enterprisedb.com> wrote: >>>> >>>>> Hi Aditya, >>>>> >>>>> Please find the attached updated patch. >>>>> >>>>> On Mon, Jul 6, 2020 at 11:44 AM Aditya Toshniwal < >>>>> aditya.toshni...@enterprisedb.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.jo...@enterprisedb.com> wrote: >>>>>> >>>>>>> Hi Aditya, >>>>>>> >>>>>>> Can you please review it. >>>>>>> >>>>>>> On Mon, Jul 6, 2020 at 11:17 AM Khushboo Vashi < >>>>>>> khushboo.va...@enterprisedb.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*
