Hi, Resending the patch. Missed the requirements.txt file in the previous patch.
Thanks, Khushboo On Wed, Apr 1, 2020 at 5:38 PM Khushboo Vashi < khushboo.va...@enterprisedb.com> wrote: > 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_v3.patch
Description: Binary data
