Re: [pgAdmin4][Patch] - RM 2186 - Support external authentication sources [LDAP]

Hi,

Please find the attached updated patch.

On Tue, Mar 17, 2020 at 4:11 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> On Tue, Mar 17, 2020 at 10:24 AM Khushboo Vashi <
> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>> Hi Dave,
>>
>> Thanks for the review.
>>
>> On Tue, Mar 17, 2020 at 3:42 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> Hi
>>>
>>> 30 second read of the first version of the patch...
>>>
>>> - Please move the configuration into config.py. Users should never have
>>> to modify a distributed file (it messes up packaging). I don't see any
>>> reason to use a different file just for auth config.
>>>
>>> There are many settings for the LDAP, and in the future we will add
>> other external sources also, so I thought it would be better if we have
>> different file for the authentication.
>>
>
> Sure, but our config file is small compared to many. Splitting things out
> is more confusing for users. If they want to do that themselves of course,
> they can add a config_local.py file which includes other files as needed.
>
Fixed.

>
>
>> - I think all config options should be prefixed with LDAP_ as we may have
>>> things like CERT_FILE for other purposes too.
>>>
>>> Sure.
>>
> Done.

> - I don't see any test cases.
>>>
>>> I will think about this, as right now no idea how to write test cases
>> for this.
>>
>
> It should be fairly straightforward to write tests for some of the
> functions in the auth classes. For testing the actual LDAP stuff, we
> probably need to add LDAP config options to test_config.json, and only if
> present, run the tests. That would probably need to support a list of LDAP
> servers, so we can test with different configurations (LDAP, LDAPS,
> LDAP_STARTTLS, AD etc).
>
>
Done.

Thanks,
Khushboo

> Thanks.
>>>
>>> Thanks,
>> Khushboo
>>
>>>
>>> On Tue, Mar 17, 2020 at 8:55 AM Khushboo Vashi <
>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi,
>>>>
>>>> Please find the attached patch to support LDAP Authentication in Server
>>>> mode.
>>>> To test the patch, config_auth.py needs to be configured for LDAP
>>>> configurations. The config settings are explained in this file in detail.
>>>> After configuring the parameters, start the pgadmin server in Server mode
>>>> and connect with LDAP server with the valid user via login page.
>>>>
>>>> I have tested this patch with ldap and ldap + ssl/tls. With the TLS, I
>>>> have used the default config of ldap3 without certificates.
>>>>
>>>> @Dave, can you please review this patch, as you have a better
>>>> understanding of LDAP and you can easily pointed out if I have missed
>>>> anything.
>>>>
>>>> Note: For the document update I will create the task and assign to
>>>> Nidhi for the same.
>>>>
>>>> Thanks,
>>>> Khushboo
>>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>