On Mon, Jan 18, 2021 at 9:37 AM Khushboo Vashi < khushboo.va...@enterprisedb.com> wrote:
> > > On Mon, Jan 18, 2021 at 2:45 PM Dave Page <dp...@pgadmin.org> wrote: > >> Hi >> >> On Mon, Jan 18, 2021 at 7:30 AM Khushboo Vashi < >> khushboo.va...@enterprisedb.com> wrote: >> >>> Hi, >>> >>> Please find the attached updated patch with the below changes: >>> >>> - Dependencies are added into Linux packages in the RPM/DEBs. >>> - Dev packages are added in the setup scripts for Linux. >>> - The required packages are added in the Dockerfile. >>> - Conditional gssapi 1.6.2 dependency is added for Python 3.5 in >>> requirements.txt. >>> >> >> 1.6.9 is the last release that supports Python 3.4+. We should use that >> rather than older versions. >> > As per the https://pypi.org/project/gssapi/*1.6.9*/, it says Requires: Python > >=3.6.* > I think that's the metadata for the latest package version on the left. If you read the main text, it says: Requirements Basic - A working implementation of GSSAPI (such as from MIT Kerberos) which includes header files - a C compiler (such as GCC) - either the enum34 Python package or Python 3.4+ - the six and decorator python package For 1.6.10, that changed to: Requirements Basic - A working implementation of GSSAPI (such as from MIT Kerberos) which supports delegation and includes header files - a C compiler (such as GCC) - Python 3.6+ (older releases support older versions, but are unsupported) - the decorator python package > >> >> >>> - krb5 libs are not bundled with the Desktop packages, so added the >>> gssapi dependency into the try/catch block. >>> - .dockerignore is introduced to ignore unwanted files/folders like >>> node_modules etc., which will make the docker build fast. (By Ashesh Vashi) >>> >> >> Aside from that one comment above, eyeball review of the build changes >> looks good. >> >> >> >>> >>> Thanks, >>> Khushboo >>> >>> On Fri, Jan 15, 2021 at 3:48 PM Dave Page <dp...@pgadmin.org> wrote: >>> >>>> And another thought... >>>> >>>> Some of the Jenkins QA jobs setup the virtual environment for running >>>> tests themselves. I believe these might actually be the cause of some of >>>> the failures we saw initially with the commit - I'll review those, and >>>> ensure they won't try to build the gssapi module from source on Windows. >>>> >>>> On Thu, Jan 14, 2021 at 4:34 PM Dave Page <dp...@pgadmin.org> wrote: >>>> >>>>> FYI, I did a quick test (and browse of PyPI): >>>>> >>>>> - On Windows, it seems there is a binary wheel available: >>>>> >>>>> (gssapi) C:\Users\dpage>pip install gssapi >>>>> Collecting gssapi >>>>> Downloading gssapi-1.6.12-cp39-cp39-win_amd64.whl (670 kB) >>>>> |████████████████████████████████| 670 kB 3.3 MB/s >>>>> Collecting decorator >>>>> Downloading decorator-4.4.2-py2.py3-none-any.whl (9.2 kB) >>>>> Installing collected packages: decorator, gssapi >>>>> Successfully installed decorator-4.4.2 gssapi-1.6.12 >>>>> >>>>> - On macOS, the wheel is built by pip, but it doesn't seem to have any >>>>> additional binary dependencies. >>>>> >>>>> This should simplify things a lot - we just need to ensure the build >>>>> scripts use the binary package on Windows, and install the build deps on >>>>> the Linux/Docker environments (and update the package builds with the >>>>> additional dependencies of course). >>>>> >>>>> >>>>> On Thu, Jan 14, 2021 at 4:04 PM Dave Page <dp...@pgadmin.org> wrote: >>>>> >>>>>> Hi Khushboo, >>>>>> >>>>>> As you know, this has been rolled back as the buildfarm blew up. I >>>>>> think there are a number of TODOs that need to be addressed, given that >>>>>> the >>>>>> gssapi Python module is dependent on MIT Kerberos: >>>>>> >>>>>> In the patch: >>>>>> >>>>>> - Linux packages will need the additional dependencies to be declared >>>>>> in the RPM/DEBs. >>>>>> - The setup scripts for Linux will need to have the -dev packages >>>>>> added as appropriate. >>>>>> - The various READMEs that describe how to build packages will need >>>>>> to be updated. >>>>>> - The Dockerfile will need to be modified to add the required >>>>>> packages. >>>>>> - The Windows build will need to be updated so the installer ships >>>>>> additional required DLLs. >>>>>> - Are there any additional macOS dependencies? If so, they need to be >>>>>> handled. >>>>>> >>>>>> In the buildfarm: >>>>>> >>>>>> - All Linux build VMs need to be updated with the additional >>>>>> dependencies. >>>>>> - On Windows, we need to figure out how to build/ship KfW. It's a >>>>>> pain to build, which we would typically do ourselves to ensure we're >>>>>> consistently using the same buildchain. If we do build it ourselves: >>>>>> - Will the Python package find it during it's build? >>>>>> - We'll need to create a Jenkins job to perform the build. >>>>>> - Is any work required on macOS, or does it ship with everything >>>>>> that's needed? If not, we'll need to build it, and create the Jenkins >>>>>> job. >>>>>> >>>>>> One final thought: on Windows/macOS, can we force a binary >>>>>> installation from PIP (pip install --only-binary=gssapi gssapi)? If so, >>>>>> will that include the required libraries, as psycopg2-binary does? >>>>>> >>>>>> >>>>>> On Thu, Jan 14, 2021 at 8:18 AM Akshay Joshi < >>>>>> akshay.jo...@enterprisedb.com> wrote: >>>>>> >>>>>>> Thanks, patch applied. >>>>>>> >>>>>>> On Thu, Jan 14, 2021 at 1:42 PM Khushboo Vashi < >>>>>>> khushboo.va...@enterprisedb.com> wrote: >>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> Please ignore my previous patch, attached the updated one. >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Khushboo >>>>>>>> >>>>>>>> On Thu, Jan 14, 2021 at 12:17 PM Khushboo Vashi < >>>>>>>> khushboo.va...@enterprisedb.com> wrote: >>>>>>>> >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> Please find the attached updated patch. >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> Khushboo >>>>>>>>> >>>>>>>>> On Thu, Jan 14, 2021 at 12:00 PM Akshay Joshi < >>>>>>>>> akshay.jo...@enterprisedb.com> wrote: >>>>>>>>> >>>>>>>>>> Hi Khushboo >>>>>>>>>> >>>>>>>>>> Seems you have attached the wrong patch. Please send the updated >>>>>>>>>> patch. >>>>>>>>>> >>>>>>>>>> On Wed, Jan 13, 2021 at 2:35 PM Khushboo Vashi < >>>>>>>>>> khushboo.va...@enterprisedb.com> wrote: >>>>>>>>>> >>>>>>>>>>> Hi, >>>>>>>>>>> >>>>>>>>>>> Please find the attached updated patch. >>>>>>>>>>> >>>>>>>>>>> Thanks, >>>>>>>>>>> Khushboo >>>>>>>>>>> >>>>>>>>>>> On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal < >>>>>>>>>>> aditya.toshni...@enterprisedb.com> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi Khushboo, >>>>>>>>>>>> >>>>>>>>>>>> I've just done the code review. Apart from below, the patch >>>>>>>>>>>> looks good to me: >>>>>>>>>>>> >>>>>>>>>>>> 1) Move the auth source constants -ldap, kerberos out of app >>>>>>>>>>>> object. They don't belong there. You can create the constants >>>>>>>>>>>> somewhere else and import them. >>>>>>>>>>>> >>>>>>>>>>>> +app.PGADMIN_LDAP_AUTH_SOURCE = 'ldap' >>>>>>>>>>>> >>>>>>>>>>>> +app.PGADMIN_KERBEROS_AUTH_SOURCE = 'kerberos' >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Done >>>>>>>>>>> >>>>>>>>>>>> 2) Are we going to make kerberos default for wsgi ? >>>>>>>>>>>> >>>>>>>>>>>> *--- a/web/pgAdmin4.wsgi* >>>>>>>>>>>> >>>>>>>>>>>> *+++ b/web/pgAdmin4.wsgi* >>>>>>>>>>>> >>>>>>>>>>>> @@ -24,6 +24,10 @@ builtins.SERVER_MODE = True >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> import config >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> + >>>>>>>>>>>> >>>>>>>>>>>> +config.AUTHENTICATION_SOURCES = ['kerberos'] >>>>>>>>>>>> >>>>>>>>>>>> +config.KERBEROS_AUTO_CREATE_USER = True >>>>>>>>>>>> >>>>>>>>>>>> + >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Removed, it was only for testing. >>>>>>>>>>> >>>>>>>>>>>> 3) Remove the commented code. >>>>>>>>>>>> >>>>>>>>>>>> + # if self.form.data['email'] and >>>>>>>>>>>> self.form.data['password'] and \ >>>>>>>>>>>> >>>>>>>>>>>> + # source.get_source_name() ==\ >>>>>>>>>>>> >>>>>>>>>>>> + # >>>>>>>>>>>> current_app.PGADMIN_KERBEROS_AUTH_SOURCE: >>>>>>>>>>>> >>>>>>>>>>>> + # continue >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Removed the comment, it is actually the part of the code. >>>>>>>>>>> >>>>>>>>>>>> 4) KERBEROSAuthentication could be KerberosAuthentication >>>>>>>>>>>> >>>>>>>>>>>> class KERBEROSAuthentication(BaseAuthentication): >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Done. >>>>>>>>>>> >>>>>>>>>>>> 5) You can use the constants (ldap, kerberos) you had defined >>>>>>>>>>>> when creating a user. >>>>>>>>>>>> >>>>>>>>>>>> + 'auth_source': 'kerberos' >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Done. >>>>>>>>>>> >>>>>>>>>>>> 6) The below URLs belong to the authenticate module. Currently >>>>>>>>>>>> they are in the browser module. I would also suggest rephrasing >>>>>>>>>>>> the URL >>>>>>>>>>>> from /kerberos_login to /login/kerberos. Same for logout. >>>>>>>>>>>> >>>>>>>>>>> Done the rephrasing as well as moved to the authentication >>>>>>>>>>> module. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> Also, even though the method GET works, we should use the POST >>>>>>>>>>>> method for login and DELETE for logout. >>>>>>>>>>>> >>>>>>>>>>> Kerberos_login just redirects the page to the actual login, so >>>>>>>>>>> no need for the POST method. >>>>>>>>>>> I followed the same method for the Logout user we have used for >>>>>>>>>>> the normal user. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> +@blueprint.route("/kerberos_login", >>>>>>>>>>>> >>>>>>>>>>>> + endpoint="kerberos_login", methods=["GET"]) >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> +@blueprint.route("/kerberos_logout", >>>>>>>>>>>> >>>>>>>>>>>> + endpoint="kerberos_logout", methods=["GET"]) >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> On Tue, Dec 22, 2020 at 6:07 PM Akshay Joshi < >>>>>>>>>>>> akshay.jo...@enterprisedb.com> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Hi Aditya >>>>>>>>>>>>> >>>>>>>>>>>>> Can you please do the code review? >>>>>>>>>>>>> >>>>>>>>>>>>> On Tue, Dec 22, 2020 at 3:44 PM Khushboo Vashi < >>>>>>>>>>>>> khushboo.va...@enterprisedb.com> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>> >>>>>>>>>>>>>> Please find the attached patch to support Kerberos >>>>>>>>>>>>>> Authentication in pgAdmin RM 5457. >>>>>>>>>>>>>> >>>>>>>>>>>>>> The patch introduces a new pluggable option for Kerberos >>>>>>>>>>>>>> authentication, using SPNEGO to forward kerberos tickets through >>>>>>>>>>>>>> a browser >>>>>>>>>>>>>> which will bypass the login page entirely if the Kerberos >>>>>>>>>>>>>> Authentication >>>>>>>>>>>>>> succeeds. >>>>>>>>>>>>>> >>>>>>>>>>>>>> The complete setup of the Kerberos Server + pgAdmin Server + >>>>>>>>>>>>>> Client is documented in a separate file and attached. >>>>>>>>>>>>>> >>>>>>>>>>>>>> This patch also includes the small fix related to logging >>>>>>>>>>>>>> #5829 >>>>>>>>>>>>>> >>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>> Khushboo >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> -- >>>>>>>>>>>>> *Thanks & Regards* >>>>>>>>>>>>> *Akshay Joshi* >>>>>>>>>>>>> *pgAdmin Hacker | Principal Software Architect* >>>>>>>>>>>>> *EDB Postgres <http://edbpostgres.com>* >>>>>>>>>>>>> >>>>>>>>>>>>> *Mobile: +91 976-788-8246* >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> -- >>>>>>>>>>>> Thanks, >>>>>>>>>>>> Aditya Toshniwal >>>>>>>>>>>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com* >>>>>>>>>>>> <http://edbpostgres.com> >>>>>>>>>>>> "Don't Complain about Heat, Plant a TREE" >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> *Thanks & Regards* >>>>>>>>>> *Akshay Joshi* >>>>>>>>>> *pgAdmin Hacker | Principal Software Architect* >>>>>>>>>> *EDB Postgres <http://edbpostgres.com>* >>>>>>>>>> >>>>>>>>>> *Mobile: +91 976-788-8246* >>>>>>>>>> >>>>>>>>> >>>>>>> >>>>>>> -- >>>>>>> *Thanks & Regards* >>>>>>> *Akshay Joshi* >>>>>>> *pgAdmin Hacker | Principal Software Architect* >>>>>>> *EDB Postgres <http://edbpostgres.com>* >>>>>>> >>>>>>> *Mobile: +91 976-788-8246* >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Dave Page >>>>>> Blog: http://pgsnake.blogspot.com >>>>>> Twitter: @pgsnake >>>>>> >>>>>> EDB: http://www.enterprisedb.com >>>>>> >>>>>> >>>>> >>>>> -- >>>>> Dave Page >>>>> Blog: http://pgsnake.blogspot.com >>>>> Twitter: @pgsnake >>>>> >>>>> EDB: http://www.enterprisedb.com >>>>> >>>>> >>>> >>>> -- >>>> Dave Page >>>> Blog: http://pgsnake.blogspot.com >>>> Twitter: @pgsnake >>>> >>>> EDB: http://www.enterprisedb.com >>>> >>>> >> >> -- >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EDB: http://www.enterprisedb.com >> >> -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EDB: http://www.enterprisedb.com
