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
