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

2020-03-17 Thread Khushboo Vashi
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


RM_2186.patch
Description: Binary data


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

2020-03-17 Thread navnath gadakh
Hi Khushboo,
   I think there is no use of

+if app is not None:
+AuthSourceRegistry.load_auth_sources()
+

in get_auth_sources() function.


On Tue, Mar 17, 2020 at 2:25 PM Khushboo Vashi <
khushboo.va...@enterprisedb.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
>


-- 
*-- Navnath*


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

2020-03-17 Thread Dave Page
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.

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

- I don't see any test cases.

Thanks.


On Tue, Mar 17, 2020 at 8:55 AM Khushboo Vashi <
khushboo.va...@enterprisedb.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


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

2020-03-17 Thread Khushboo Vashi
Hi Dave,

Thanks for the review.

On Tue, Mar 17, 2020 at 3:42 PM Dave Page  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.

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

> - I don't see any test cases.
>
> I will think about this, as right now no idea how to write test cases for
this.

> Thanks.
>
> Thanks,
Khushboo

>
> On Tue, Mar 17, 2020 at 8:55 AM Khushboo Vashi <
> khushboo.va...@enterprisedb.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
>


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

2020-03-17 Thread Khushboo Vashi
Hi Navnath,

On Tue, Mar 17, 2020 at 3:37 PM navnath gadakh <
navnath.gad...@enterprisedb.com> wrote:

> Hi Khushboo,
>I think there is no use of
>
> +if app is not None:
> +AuthSourceRegistry.load_auth_sources()
> +
>
> in get_auth_sources() function.
>
> Thanks for the review, I will look into it.

Thanks,
Khushboo

>
> On Tue, Mar 17, 2020 at 2:25 PM Khushboo Vashi <
> khushboo.va...@enterprisedb.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
>>
>
>
> --
> *-- Navnath*
>


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

2020-03-17 Thread Dave Page
Hi

On Tue, Mar 17, 2020 at 10:24 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi Dave,
>
> Thanks for the review.
>
> On Tue, Mar 17, 2020 at 3:42 PM Dave Page  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.


> - I think all config options should be prefixed with LDAP_ as we may have
>> things like CERT_FILE for other purposes too.
>>
>> Sure.
>
>> - 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).


> Thanks.
>>
>> Thanks,
> Khushboo
>
>>
>> On Tue, Mar 17, 2020 at 8:55 AM Khushboo Vashi <
>> khushboo.va...@enterprisedb.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