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 1l05aT-0008K2-BS for pgadmin-hackers@arkaria.postgresql.org; Thu, 14 Jan 2021 16:35:05 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1l05aR-0001eX-8P for pgadmin-hackers@arkaria.postgresql.org; Thu, 14 Jan 2021 16:35:03 +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 1l05aR-0001eQ-0L for pgadmin-hackers@lists.postgresql.org; Thu, 14 Jan 2021 16:35:03 +0000 Received: from mail-ed1-x52e.google.com ([2a00:1450:4864:20::52e]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1l05aM-0008AR-Q3 for pgadmin-hackers@postgresql.org; Thu, 14 Jan 2021 16:35:01 +0000 Received: by mail-ed1-x52e.google.com with SMTP id h16so6360827edt.7 for ; Thu, 14 Jan 2021 08:34:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Q9ylIPPFCawM9tps8SiwyuwT7I1GwY3tQL79bXf2aLo=; b=EYDnCo2Ze+VVOeDpducUsVCuODZz7OSyk/sDV38VzH48LtCyKJUpD0QBvUH1zdgteS 0tqawtEio1yz2Zlt8p6cauCjEIsGtyRpLdF6wbQ6DvfxwGLRrSMsdE5k61b/WhayrAZ0 C9qV2GB4wrAuSebzZdWHVmcBVrkad6eEd6jc2Y/hmk3BFvhcvfmfbWvxyIRvzcZVSLYp z4hPeJuvcG6au8NsoUMKW7PEZVa1FUxw4aDcodvx0vYHtyjTF+ccrz3vObdzTacsa3yo ZJ/afv7jMlSi1ZQU485+vrRuzG5h5jypEv64RfAUw+ITBTp4RvUEx5GhjXIhWlRq8SnA 4klw== 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=Q9ylIPPFCawM9tps8SiwyuwT7I1GwY3tQL79bXf2aLo=; b=tym4Ak1MRYhGfpucda7GXj8NAFCSwMSaejtP6UJqjsMzkq4moaIGWZRFJCLSWtBqY7 Y5aw+0XypUJ1mLnx72PjYuEeLvmM2B9JJQ2xZmw9UeVqUrA9FhirvyvOQNPbHgApxzhV u62MFzfRU3Sr4w/gtaj8lhJXW8IPKtGqkCDcBvNUhgSkoQlK738/DyzaJ4SshOyKqFTo 3uZ0URVETsQx33hZ808KwMH/JXUpMCY562DcJS5Mq3xqSBBeMXYlJFbELCsuXEFJ5sx+ PbAymcaNgD7xcdB9+pP0WSqfJdBVvR/C+U84/LBBGpRvdERroy4sJhYAnJoVBZMVEP0P 5BlQ== X-Gm-Message-State: AOAM532QW7eecJwAD93znPAkaoPRiGIWo0BNYVOuFQMxohYzmvxl/Owy gKpvBmgDScfiRWJiK40hbxmp0YY1QYzWmXbr7IYWOQ== X-Google-Smtp-Source: ABdhPJy/MqPOMxfXlYdP3wONXEhs9Fg0I34Ag90S6lVYOi6exp5U/U6oNpzOidbAO7dXHk4zhT9l0XbszgMhQS4YDDw= X-Received: by 2002:a50:d88c:: with SMTP id p12mr6379085edj.370.1610642096510; Thu, 14 Jan 2021 08:34:56 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Dave Page Date: Thu, 14 Jan 2021 16:34:45 +0000 Message-ID: Subject: Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1 To: Akshay Joshi Cc: Khushboo Vashi , Aditya Toshniwal , pgadmin-hackers Content-Type: multipart/alternative; boundary="00000000000005ea6305b8ded68c" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --00000000000005ea6305b8ded68c Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 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 a= s > 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=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 patc= h. >>>>> >>>>> 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 els= e 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 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=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 through = a browser >>>>>>>>> which will bypass the login page entirely if the Kerberos Authent= ication >>>>>>>>> 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 * >>>>>>>> >>>>>>>> *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 > > --=20 Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EDB: http://www.enterprisedb.com --00000000000005ea6305b8ded68c Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
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
=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 decorator
=C2=A0 Download= ing decorator-4.4.2-py2.py3-none-any.whl (9.2 kB)
Installing collected p= ackages: 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.
<= div>
This should simplify things a lot - we just need to ensu= re the build scripts use the binary package on Windows, and install the bui= ld deps on the Linux/Docker environments=C2=A0(and update the package build= s 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 kn= ow, 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 mo= dule is dependent on MIT Kerberos:

In the patch:

- Linux packages will need the additional dependenc= ies 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 p= ackages.
- The Windows build will need to be updated so the insta= ller ships additional required DLLs.
- Are there any additional m= acOS dependencies? If so, they need to be handled.

In the buildfarm:

- All Linux build VMs need to b= e 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 wo= uld typically do ourselves to ensure we're consistently using the same = buildchain. If we do build it ourselves:
=C2=A0 - Will the Python= package find it during it's build?
=C2=A0 - We'll need t= o create a Jenkins job to perform the build.
- Is any work requir= ed on macOS, or does it ship with everything that's needed? If not, we&= #39;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 incl= ude the required libraries, as psycopg2-binary=C2=A0does?


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

Hi,=C2=A0
<= br>
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:
<= div>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=C2=A0Khush= boo

Seems you have attached the wrong patch. Please send= the updated patch.

On Wed, Jan 13, 2021 at 2:35 PM Khushboo Vashi <= ;khush= boo.vashi@enterprisedb.com> wrote:
Hi,

Please find the attached updated patch.

Th= anks,
Khushboo

On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniw= al <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, k= erberos out of app object. They don'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?



--


--
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



--


--
--00000000000005ea6305b8ded68c--