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 1l056j-0007R4-Ii for pgadmin-hackers@arkaria.postgresql.org; Thu, 14 Jan 2021 16:04:22 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1l056i-0002K9-Go for pgadmin-hackers@arkaria.postgresql.org; Thu, 14 Jan 2021 16:04:20 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1l056h-0002JI-GK for pgadmin-hackers@lists.postgresql.org; Thu, 14 Jan 2021 16:04:20 +0000 Received: from mail-ed1-x52f.google.com ([2a00:1450:4864:20::52f]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1l056d-0006R8-Lk for pgadmin-hackers@postgresql.org; Thu, 14 Jan 2021 16:04:18 +0000 Received: by mail-ed1-x52f.google.com with SMTP id r5so6232756eda.12 for ; Thu, 14 Jan 2021 08:04:15 -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=D06s8pLqfH40b3T7e5aRvd1bQ4Ugvlj2oX+K1w9HOrY=; b=ci49IBA5HPuErNJ+w6iKDmz4jRG4KdDr9SxHGE8NQoCjiMmfyZdHJLNEhw2pU8ANtJ 3vOlD5wPngl3ElAUSik7XRRMcCsWqun/3eJsM4FdtlE22sBCAqt3oPtGu1m6FS8WBcTd djT9XYczkBYlYhO9jc5CG7teQ2WD2zWxxmFG0XQDuwxdS91YLUILk4f8hEfgX07O4Gnh N/W3z2HtOwDEIVpNDgI3CyA5yRaB1UP7dEWsUoJhrO1KiSz5nmw9/HsrxdaHkhYcj5N6 YWHPYUsUR51ErcjNmQLwKwv+ELpHxB+VsbIpyqnuZDtK5/KkjAdRFUPqA1FDovf5bJra X17g== 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=D06s8pLqfH40b3T7e5aRvd1bQ4Ugvlj2oX+K1w9HOrY=; b=g5JABTcPv6+qiNW0OWR5wOg1XwFyWwLYbBiNMfhHI2iNx4rgkEt+pNsRvNH3XX2UX5 CGo5hpZhmH0LVJ40AH9KQ2rtHdHg43JOsF6VJItP0liV+Q6ItWxk3y9361K6EdABSScW kw6M6jVUZgERTjuqwgxs4Cef3FJsg5G5z+wzPQqr4/UtJ9LhLc/IbjmHmkVNDIk+6j+T OakYklR5rN6yyijR0T4ryUF4mf4cJ6380Bd/iLa38WrE1HX7f8YWG3hzrMBmLlyabjEW n+sEiyPgAXtarTaG1Jc9z/vJbGYDmEli2rnp5fHI9hrwmkuqy+gbzOBf+Qzb4l0cvDHb Q2dQ== X-Gm-Message-State: AOAM531TTrlxcBDgVlCdeOBxO4Du46s61yn0EflfFThNxADYAanUrJo2 I+leBcLr5C/m63hj/fCUV6s7RSxoIZWa7wRmP6cpFA== X-Google-Smtp-Source: ABdhPJzRJeGjyRaK9xrXDxzxf9cjMDi3blXre+hiPbNP28sRJN8dQK3UbIfzwdCAw7uBUGpe8raArWrososgAIHee6Q= X-Received: by 2002:a50:bc06:: with SMTP id j6mr6595357edh.235.1610640253167; Thu, 14 Jan 2021 08:04:13 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Dave Page Date: Thu, 14 Jan 2021 16:04:02 +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="00000000000026cb3505b8de6827" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --00000000000026cb3505b8de6827 Content-Type: text/plain; charset="UTF-8" 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 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 = '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.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 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 * >>>>>>> >>>>>>> *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 --00000000000026cb3505b8de6827 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
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 declare= d in the RPM/DEBs.
- The setup scripts for Linux will need to hav= e the -dev packages added as appropriate.
- The various READMEs t= hat 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 additi= onal 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 th= e additional dependencies.
- On Windows, we need to figure out ho= w 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:
=C2=A0 - Will the Python package find it = during it's build?
=C2=A0 - We'll need to create a Jenkin= s job to perform the build.
- Is any work required on macOS, or d= oes it ship with everything that's needed? If not, we'll need to bu= ild it, and create the Jenkins job.

One final thou= ght: on Windows/macOS, can we force a binary installation from PIP (pip ins= tall --only-binary=3Dgssapi gssapi)? If so, will that include 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 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



--
--00000000000026cb3505b8de6827--