Please disregard my previous patch, attached the updated patch. :)
On Tue, Mar 24, 2020 at 10:32 AM Khushboo Vashi < khushboo.va...@enterprisedb.com> wrote: > Please disregard my previous patch, attached the updated patch. > > On Tue, Mar 24, 2020 at 10:29 AM Khushboo Vashi < > khushboo.va...@enterprisedb.com> wrote: > >> Hi, >> >> Please find the attached updated patch. >> >> >> On Tue, Mar 17, 2020 at 4:11 PM Dave Page <dp...@pgadmin.org> wrote: >> >>> 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 <dp...@pgadmin.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.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 >>> >>
RM_2186_v1.patch
Description: Binary data
