Hi, Please find the attached updated patch which includes the review comments given in the review meeting:
1. Do not store password for ldap user in sqlite database 2. Forgot Password : Give error to ldap users 3. User Management dialog changes 4. Authentication source display besides username / email after login Thanks, Khushboo On Tue, Mar 24, 2020 at 3:20 PM Khushboo Vashi < khushboo.va...@enterprisedb.com> wrote: > 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_v2.patch
Description: Binary data
