Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1l1Qz3-0002Ir-Jj for pgadmin-hackers@arkaria.postgresql.org; Mon, 18 Jan 2021 09:38:02 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1l1Qz2-0008SW-91 for pgadmin-hackers@arkaria.postgresql.org; Mon, 18 Jan 2021 09:38:00 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1l1Qz1-0008SN-W6 for pgadmin-hackers@lists.postgresql.org; Mon, 18 Jan 2021 09:38:00 +0000 Received: from mail-lf1-x12b.google.com ([2a00:1450:4864:20::12b]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1l1Qyx-0000IC-Tv for pgadmin-hackers@postgresql.org; Mon, 18 Jan 2021 09:37:59 +0000 Received: by mail-lf1-x12b.google.com with SMTP id o13so23127381lfr.3 for ; Mon, 18 Jan 2021 01:37:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=OHGTP7Aoqn5PVt9CcRsnAfJ4J+UQDEf43yz/rJwwVBA=; b=yTcS0tIF7FhIopCC/Oz3n/CYAtpOX8Cy1yO6RobWWrmoMAErRZ0ofl2bmDrxbsFUCS ajPEDREZHcE8Pc2WCpiCVzjrEIpwt83KE4blKnSMFxhU5KIyTaEptyk7sfFFNasiRrm/ JDU2HkxoeCu4CtG5B2cQQb0lFeHdV+aGg+hsikZrUGpZVktPRX59IdqlgG+IE8mudFoU YHkyzTyorXvwk5DwGj3HLyj4xnMv4IL7ZL2EEHhEhEnEeQf7981XtMDtsKbrbsiVaf3H DicWu/ukf8DVtJt5vNFouVTREMc/adgTGRyJFLJ/+bMjNRMHfa1tLO5bz69mEUE794U7 k/OQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=OHGTP7Aoqn5PVt9CcRsnAfJ4J+UQDEf43yz/rJwwVBA=; b=PMz0KBuR4b/82h/iHL30mrEfWcb0+vbHhC8QXkVk0M+drZpKX4rzRHPX0x8zGak0gC HhR6jkgEKEOEjW1byhASfiUYfdbtH5K+8zo8PzFtixAZSEmOkufUjOd6xKyfbch73gmr WSIsm2uoruywX1p7KvXJ8NAehXHLMK3rxIy8Sn9JrP1g53H9nwZnJBxL/LitcoKVs4I9 Qe3RdbjQqlNDgnucX5c8oSOA2rgD/UGFV+6X4IoCR657PGTTBDdrJy9QLpgGAnmqF98k Y6LL3eqDljtiJE7GCfyivnGMGB8z3Rjfaz9D1y1k1Zo+SfKZys8FHpOoUM9quzN1FqE/ mhlg== X-Gm-Message-State: AOAM530/YxK8w21k+pOKDXXwJ8SOE75EYF3BskLQqPGkwP0K2qkwgYW3 VVaQ0Q9uxvmg3YEKCVOvjRnt6lCOx7B8YJc/86NzhuOHeO6yQf7q4t9iTHwpjh5OPXgKqoPjrRX NM4n+DUiGMCErR9f9hnUJEtSXEciPa+pGqV7m6yNDblYbDCSr/zaCGtSkWlqnusmmGF6y5Qd2r1 JMvNvHgSKds7vhVJcW6ifKVurXE8sQANW8YMyFkVj4PFKc65gtXk1Vb+jVWQ== X-Google-Smtp-Source: ABdhPJzp0z2gzWA+qqRfzDwSlrECAraFJW7rG2z0PF65pGr4qTuqtJGcmuU6TcJS0IWcgzAvXFmvUmYamSTNmW+I9+0= X-Received: by 2002:a05:6512:6d6:: with SMTP id u22mr11899120lff.454.1610962674531; Mon, 18 Jan 2021 01:37:54 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Khushboo Vashi Date: Mon, 18 Jan 2021 15:07:52 +0530 Message-ID: Subject: Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1 To: Dave Page Cc: Akshay Joshi , Aditya Toshniwal , pgadmin-hackers Content-Type: multipart/alternative; boundary="000000000000f6538405b929790a" X-CLOUD-SEC-AV-Info: enterprisedb,google_mail,monitor X-CLOUD-SEC-AV-Sent: true X-Gm-Spam: 0 X-Gm-Phishy: 0 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000f6538405b929790a Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, Jan 18, 2021 at 2:45 PM Dave Page wrote: > Hi > > On Mon, Jan 18, 2021 at 7:30 AM Khushboo Vashi < > khushboo.vashi@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: Pyth= on >=3D3.6.* > > >> - 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 Vas= hi) >> > > 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 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 o= f >>> 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 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) >>>> |=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88= =E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2= =96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96= =88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88= | 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 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 th= at 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 t= o >>>>> be updated. >>>>> - The Dockerfile will need to be modified to add the required package= s. >>>>> - 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 pai= n >>>>> 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=3Dgssapi 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.joshi@enterprisedb.com> wrote: >>>>> >>>>>> Thanks, patch applied. >>>>>> >>>>>> On Thu, Jan 14, 2021 at 1:42 PM Khushboo Vashi < >>>>>> khushboo.vashi@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.vashi@enterprisedb.com> wrote: >>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> Please find the attached updated patch. >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Khushboo >>>>>>>> >>>>>>>> On Thu, Jan 14, 2021 at 12:00 PM Akshay Joshi < >>>>>>>> akshay.joshi@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.vashi@enterprisedb.com> wrote: >>>>>>>>> >>>>>>>>>> Hi, >>>>>>>>>> >>>>>>>>>> Please find the attached updated patch. >>>>>>>>>> >>>>>>>>>> Thanks, >>>>>>>>>> Khushboo >>>>>>>>>> >>>>>>>>>> On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal < >>>>>>>>>> aditya.toshniwal@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 =3D 'ldap' >>>>>>>>>>> >>>>>>>>>>> +app.PGADMIN_KERBEROS_AUTH_SOURCE =3D '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 =3D True >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> import config >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> + >>>>>>>>>>> >>>>>>>>>>> +config.AUTHENTICATION_SOURCES =3D ['kerberos'] >>>>>>>>>>> >>>>>>>>>>> +config.KERBEROS_AUTO_CREATE_USER =3D 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() =3D=3D\ >>>>>>>>>>> >>>>>>>>>>> + # 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 modul= e. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> 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 n= o >>>>>>>>>> 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=3D"kerberos_login", methods=3D["GET"= ]) >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> +@blueprint.route("/kerberos_logout", >>>>>>>>>>> >>>>>>>>>>> + endpoint=3D"kerberos_logout", methods=3D["GET= "]) >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> On Tue, Dec 22, 2020 at 6:07 PM Akshay Joshi < >>>>>>>>>>> akshay.joshi@enterprisedb.com> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi Aditya >>>>>>>>>>>> >>>>>>>>>>>> Can you please do the code review? >>>>>>>>>>>> >>>>>>>>>>>> On Tue, Dec 22, 2020 at 3:44 PM Khushboo Vashi < >>>>>>>>>>>> khushboo.vashi@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 thro= ugh a browser >>>>>>>>>>>>> which will bypass the login page entirely if the Kerberos Aut= hentication >>>>>>>>>>>>> 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 #58= 29 >>>>>>>>>>>>> >>>>>>>>>>>>> Thanks, >>>>>>>>>>>>> Khushboo >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> -- >>>>>>>>>>>> *Thanks & Regards* >>>>>>>>>>>> *Akshay Joshi* >>>>>>>>>>>> *pgAdmin Hacker | Principal Software Architect* >>>>>>>>>>>> *EDB Postgres * >>>>>>>>>>>> >>>>>>>>>>>> *Mobile: +91 976-788-8246* >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> -- >>>>>>>>>>> Thanks, >>>>>>>>>>> Aditya Toshniwal >>>>>>>>>>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com* >>>>>>>>>>> >>>>>>>>>>> "Don't Complain about Heat, Plant a TREE" >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> *Thanks & Regards* >>>>>>>>> *Akshay Joshi* >>>>>>>>> *pgAdmin Hacker | Principal Software Architect* >>>>>>>>> *EDB Postgres * >>>>>>>>> >>>>>>>>> *Mobile: +91 976-788-8246* >>>>>>>>> >>>>>>>> >>>>>> >>>>>> -- >>>>>> *Thanks & Regards* >>>>>> *Akshay Joshi* >>>>>> *pgAdmin Hacker | Principal Software Architect* >>>>>> *EDB Postgres * >>>>>> >>>>>> *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 > > --000000000000f6538405b929790a Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Mon, Jan 18, 2021 at 2:45 PM Dave = Page <dpage@pgadmin.org> wro= te:
Hi

On Mon, Jan 18, 2021 at 7:30 AM Khushboo Vashi <= khushb= oo.vashi@enterprisedb.com> wrote:
Hi,

Please fin= d 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.
- T= he required packages are added in the Dockerfile.
- Condition= al gssapi 1.6.2 dependency is added for Python 3.5 in requirements.txt.

1.6.9 is the last release th= at supports Python 3.4+. We should use that rather than older versions.
As per the=C2=A0https://pypi.org/project/gssapi/1.6.9/, it say= s=C2=A0Requires:<= span style=3D"background-color:rgb(253,253,253);color:rgb(70,70,70);font-fa= mily:"Source Sans Pro",Helvetica,Arial,sans-serif">=C2=A0<= span style=3D"background-color:rgb(253,253,253);color:rgb(70,70,70);font-fa= mily:"Source Sans Pro",Helvetica,Arial,sans-serif">Python >=3D= 3.6.*
=C2=A0
=C2=A0
-= krb5 libs are not bundled with the Desktop packages, so added the gssapi d= ependency into the try/catch block.=C2=A0
- .dockerignore is= introduced to ignore unwanted files/folders like node_modules etc., which = will make the docker build fast.=C2=A0(By Ashesh=C2=A0Vashi)

Aside from that one comment above= , eyeball review of the build changes looks good.

= =C2=A0

Thanks,
Khushboo

On Fri, Jan 15, 2021 at 3:48 PM Dave Page <dpage@pgadmin.org&g= t; 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 <dpage@pgadmin.org> wrote:
FYI, I did a quick test (and browse= of PyPI):

- On Windows, it seems there is a binary whee= l available:

(gssapi) C:\Users\dpage>pip instal= l gssapi
Collecting gssapi
=C2=A0 Downloading gssapi-1.6.12-cp39-cp39= -win_amd64.whl (670 kB)
=C2=A0 =C2=A0 =C2=A0|=E2=96=88=E2=96=88=E2=96=88= =E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2= =96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96= =88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88= =E2=96=88=E2=96=88=E2=96=88=E2=96=88| 670 kB 3.3 MB/s
Collecting decorat= or
=C2=A0 Downloading decorator-4.4.2-py2.py3-none-any.whl (9.2 kB)
I= nstalling collected packages: decorator, gssapi
Successfully installed d= ecorator-4.4.2 gssapi-1.6.12

- On macOS, the w= heel 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=C2=A0(and upd= ate the package builds with the additional dependencies of course).


On Thu, Jan 14, 2021 at 4:04 PM Dave Page <dpage@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 add= ressed, given that the gssapi Python module is dependent on MIT Kerberos:

In the patch:

- Linux pack= ages will need the additional dependencies to be declared in the RPM/DEBs.<= /div>
- 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 ne= ed to be modified to add the required packages.
- The Windows bui= ld 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 depend= encies.
- 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 oursel= ves:
=C2=A0 - Will the Python package find it during it's bui= ld?
=C2=A0 - We'll need to create a Jenkins job to perform th= e build.
- Is any work required on macOS, or does it ship with ev= erything that's needed? If not, we'll need to build it, and create = the Jenkins job.

One final thought: on Windows/mac= OS, can we force a binary installation from PIP (pip install --only-binary= =3Dgssapi gssapi)? If so, will that include the required libraries, as psyc= opg2-binary=C2=A0does?


On Thu, Jan 14, 2021 at 8:18 AM = Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Thanks, patch applie= d.

On Thu, Jan 14, 2021 at 1:42 PM Khushboo Vashi <khushboo.vashi@enterpris= edb.com> wrote:
Hi,=C2=A0

Please ignore my previ= ous patch, attached the updated one.

Thanks,
=
Khushboo

On Thu, Jan 14, 2021 at 12:17 PM Khushboo Vashi <khushboo.v= ashi@enterprisedb.com> wrote:
Hi,

Ple= ase find the attached updated patch.

Thanks,
=
Khushboo

On Thu, Jan 14, 2021 at 12:00 PM Akshay Joshi <akshay.joshi@enterpr= isedb.com> wrote:
Hi=C2=A0Khushboo

Seems you h= ave attached the wrong patch. Please send the updated patch.
On Wed, J= an 13, 2021 at 2:35 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com>= wrote:
Hi,

Please find the attached u= pdated patch.

Thanks,
Khushboo

On F= ri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.c= om> wrote:
Hi Khushbo= o,

I've just done the code review. Apa= rt from below, the patch looks good to me:

= 1) Move the auth source constants -ldap, kerberos out of app object. They d= on't belong there. You can create the constants somewhere=C2=A0else and= import them.

+app.PGADMIN_LDAP_AUTH_SOURCE =3D 'ldap'

+app.PGADMIN_KERBEROS_AUTH_SOURCE =3D 'kerberos'


Done=C2= =A0

2) A= re we going to make kerberos default for wsgi ?

--- a/web/pgAdmin4.wsgi

+++ b/web/pgAdmin4.wsgi

@@ -24,6 +24,10 @@<= /span> builtins.= SERVER_MODE =3D True

=C2=A0

=C2=A0import config

=C2=A0

+

+config.AUTHENTICATION_SOURCES =3D ['kerberos']

+config.KERBEROS_AUTO_C= REATE_USER =3D True

+


Removed, it was only for testing.=C2=A0

3) Remove the commented code.

+=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 # if self.form.data['email'] and self.form= .data['password'] and \

+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 # =C2=A0 =C2=A0 =C2=A0 =C2=A0 source.get_source_name() =3D=3D\=

+=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 # =C2=A0 =C2=A0 =C2=A0 =C2=A0 = current_app.PGADMIN_KERBEROS_AUTH_SOURCE:

+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 # =C2=A0 =C2=A0 continue


=
Removed the comment, it is actually the part = of the code.=C2=A0
<= div dir=3D"ltr">

class KERBEROSAuthentication(BaseAuthentication):


Done.=C2=A0
=

5) You can use the = constants (ldap, kerberos) you had defined when creating a user.

+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 'auth_source': 'kerberos'


Done.=C2=A0=

6) The below URL= s belong to the authenticate module. Currently they are in the browser modu= le. I would also suggest rephrasing the URL from /kerberos_login to /login/= kerberos. Same for logout.
Done the re= phrasing as well as moved to the authentication module.
=C2=A0
= 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 actua= l login, so no need for the POST method.
I followed the same meth= od for the Logout user we have used for the normal user.
=C2=A0

+@blueprint.route("/ke= rberos_login",

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 endpoint=3D"kerberos_login", methods=3D["G= ET"])


+@blueprint.route("/kerberos_logout",

<= p style=3D"font-family:Menlo;margin:0px;font-variant-numeric:normal;font-va= riant-east-asian:normal;font-stretch:normal;font-size:16px;line-height:norm= al;color:rgb(57,192,38)">

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 endpoint=3D"kerberos_logout", methods=3D["= GET"])



=C2=A0
On Tue, = Dec 22, 2020 at 6:07 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrot= e:
Hi Aditya

Can you please do the code review?

On= Tue, Dec 22, 2020 at 3:44 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com<= /a>> wrote:
<= div dir=3D"ltr">Hi,

Please find the attached patch to su= pport Kerberos Authentication in pgAdmin RM 5457.

= The patch introduces a new pluggable option for Kerberos authentication, us= ing=C2=A0SPNEGO=C2=A0to forward kerberos tickets through a browser which wi= ll bypass the login page entirely if the Kerberos Authentication succeeds.<= /div>

The complete setup of the Kerberos Server=C2=A0+ p= gAdmin Server=C2=A0+ Client is documented in a separate file and attached.<= /div>

This patch also includes the small fix related to = logging #5829

Thanks,
Khushboo


--


--
Thanks,
Aditya Toshniwal=
pgAdmin hacker=C2=A0| Sr. Software Engineer | edbpostgres.com
&quo= t;Don't Complain about Heat, Plant a TREE"


--
Thank= s & Regards
Akshay Joshi
pgAdmin Hacker | Principal Softw= are Architect
EDB Po= stgres
Mobile: +91 976-788-8246



--
Thank= s & Regards
Akshay Joshi
pgAdmin Hacker | Principal Softw= are Architect
EDB Po= stgres
Mobile: +91 976-788-8246



--


--


--


--
--000000000000f6538405b929790a--