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
