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 1kzxpn-00029G-20 for pgadmin-hackers@arkaria.postgresql.org; Thu, 14 Jan 2021 08:18:23 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1kzxpm-0005UT-09 for pgadmin-hackers@arkaria.postgresql.org; Thu, 14 Jan 2021 08:18:22 +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 1kzxpl-0005UL-I0 for pgadmin-hackers@lists.postgresql.org; Thu, 14 Jan 2021 08:18:21 +0000 Received: from mail-io1-xd32.google.com ([2607:f8b0:4864:20::d32]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1kzxpi-0002xa-VL for pgadmin-hackers@postgresql.org; Thu, 14 Jan 2021 08:18:20 +0000 Received: by mail-io1-xd32.google.com with SMTP id o6so9502759iob.10 for ; Thu, 14 Jan 2021 00:18:18 -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=vd/joBfPe6+6m5MZK13PvJMWDaQQPJr/LFgTnaq+/eQ=; b=1wzFeHUPJDW5iNLo5akYeB5DQCtNX975WhbJv5Seu8z7wuoQZcZbX18khXccKmBkrM A7ktlZKN/Eg2BVRwUNTutCJY2cTUV50ePKVkYuU/b/gpIQ5oCJ7sJcSdm9+x56kBNOVQ pQ7xgu/z3uwobV559g+aZe8P9eW2j01oTK0mk/CO11iU11XZ2EwTuEss1dth2p8Au74x albo6qn6ZT7rB3pydfJ5p2SQKpD5VvBmOvIku6/SYZZhDXmBjtRndEvkhMZ9UANoj5Yz gSBAp8Jy8lqG04V6C9JO3i3mRzfsV4AAEm4bJfHFChJngTjBHZwRLchuVazElf31fuer 2BoA== 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=vd/joBfPe6+6m5MZK13PvJMWDaQQPJr/LFgTnaq+/eQ=; b=l/jnVgP/OJRNMXlK80e8ioTpA3USdQ+vTmjavw+24QIa9A2TueMI0Q7xQNm0qBURwT 7gPLrFQFuwsbdtnwK0MMVJaFYh18PnXpN7rfwq57kjTvWLPwzCvKuGv7kw1Ei2x5EvlV gnJNAbI1Zhp5pRDA81gY06+8505NEsApjmfKIMhNrCsjNozhb8b5RNkhZJxW6V4Fqzqn XVNc/2U95GsM0X16jlgWgL3iLzrLTRumWB9DDbsA5I2Ua4zbrC9CwMMG7IrnJ+86IS7Z b2JMTXoAx6fbP9MB9Y1aJYxCcSUWJDRKrJi7SsuEYJvv0l8tyPUPaV1U6ngPSGGSLFz8 ZF8A== X-Gm-Message-State: AOAM530jw8KQzKDuvDtkWj31U3DsXqNuTy00lBMG9v0VC9/kcJ6HJ0bM XA3Du3gEd6hVUW1a1Qjzu6tryhqqg85WVOkfAf77m9yZ2OkUp8d9/AYb+IPse08OtGu0aynBTez x0ulglDDVvpQdo3vjGnY6FKE4mZLNEjWilFkaHpRga6+G4lEBr9bPK8foLK0segY02LvYX7B0Zt K+qzRS8wBtruOpKq5CRFk/8NS3tyDxRYsYJBlXEF1+8M0CRj3lm+gJzk0atW3mI0nPxQ== X-Google-Smtp-Source: ABdhPJxDTq4lQkkAlw5+FJtBlDHsA7xZBZnuh7K3ZnenoaKdfH/IEc4qzjVI2Q5TKor4KDhIKq1p1IBXrPjAUWQWK1w= X-Received: by 2002:a02:690e:: with SMTP id e14mr5656318jac.7.1610612297851; Thu, 14 Jan 2021 00:18:17 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Akshay Joshi Date: Thu, 14 Jan 2021 13:48:06 +0530 Message-ID: Subject: Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1 To: Khushboo Vashi Cc: Aditya Toshniwal , pgadmin-hackers Content-Type: multipart/alternative; boundary="000000000000e2824805b8d7e5c2" 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 --000000000000e2824805b8d7e5c2 Content-Type: text/plain; charset="UTF-8" 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* --000000000000e2824805b8d7e5c2 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Thanks, patch applied.

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

Ple= ase ignore my previous patch, attached the updated one.

Thanks,
Khushboo

=
On Thu, Jan 14, 2021 at 12:17 PM Khus= hboo 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 Jos= hi <a= kshay.joshi@enterprisedb.com> wrote:
Hi=C2=A0Khushboo

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@enterpr= isedb.com> wrote:
Hi,

Please = find the attached updated patch.

Thanks,
Khushboo

On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal <aditya.tosh= niwal@enterprisedb.com> wrote:
Hi Khushboo,
<= br>
I've just done t= he 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 some= where=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



--
Thanks & Regards
Akshay Joshi
pgAdmi= n Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246

--000000000000e2824805b8d7e5c2--