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 1l1QdJ-0001K6-Uu for pgadmin-hackers@arkaria.postgresql.org; Mon, 18 Jan 2021 09:15:34 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1l1QdI-000193-PN for pgadmin-hackers@arkaria.postgresql.org; Mon, 18 Jan 2021 09:15:32 +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 1l1QdI-00018w-GR for pgadmin-hackers@lists.postgresql.org; Mon, 18 Jan 2021 09:15:32 +0000 Received: from mail-ej1-x633.google.com ([2a00:1450:4864:20::633]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1l1QdF-000070-3G for pgadmin-hackers@postgresql.org; Mon, 18 Jan 2021 09:15:32 +0000 Received: by mail-ej1-x633.google.com with SMTP id rv9so3835832ejb.13 for ; Mon, 18 Jan 2021 01:15:28 -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=zmcRTnfXsmdoFSLKTukrstd+vY5JRgrvTKmjXtvJYKk=; b=ahJF7NyjPWxkPllF0xEMMAsX9Y0JsvU5daAivyOFwkHeSd4cpwtQ9BDvQaQafUQYLf iFl2TOroNHSorDdfOAiHChjsBtL+caQ9ck0opATTR6VcXVNhDAytey+CnrW0T456/nne Lj86VcQnbCWKoGoUUR9H0gkXecikque/T+Mwfc/zJh02TDkPPEj9UzDmttW2kO31EIkH icvLfjlhFZ66F4bSmepTfpF3aHgCpBwzqXMt+l2d8u+owq/RlGG4DWSGTbDLz3ED6IiR e7MdhUNR0YUdhWS86I0LzmSqNj9zGuAS8CjjA0lpYq/4vYwNb+/+0sr/lfDBRmU/XFJD 0UXw== 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=zmcRTnfXsmdoFSLKTukrstd+vY5JRgrvTKmjXtvJYKk=; b=iJnaCfpoi9P2YW4ChNLKz4kmM//vbA3FATC7t2vSCMZrPa9I504j6/LaCsL5BVty4R izRq0895l/+fVACc40SJmqRSEhlVW5a0jHHSbhuwOIHlPTbCJhqqeyZyRD0tbu4Xr7tJ NiyNNtaMRndqLORw8Jw+OvBjyjS0G8wZCJ+cNlSOGDtI+UEgBQ70Mk3Grqt7gu8YX3m8 MrxqZcajoe7ZXOjOdzhmSt1rIKk3Tx/OnryrJ4pHgtzSZuDWwGcQqMw/KDDTT5Ic8VLQ jiYrC0YQ6Gux8ihQgt+PbeL+q8slUYGlMSygFiKVrlozgVCPA65EPJH1hVcROlXb+FnL N5dQ== X-Gm-Message-State: AOAM531954bXS/U9Tf/jGDp7Y1Qc82Z0sjKRt045Rs1HN51Jg38vwLAR VGqFUU9aI9xKVaxuGCijPjsKg9VJGYktMsYiGKtgjg== X-Google-Smtp-Source: ABdhPJy6F6U6zaZuBWu39kO3eWhzT9ywmB/MphROXqllDpJYgOVnFcRzhk/JR2hvuDDK+fYF1eG0tLe93gDV2lF4Oww= X-Received: by 2002:a17:906:1a14:: with SMTP id i20mr17360184ejf.548.1610961326460; Mon, 18 Jan 2021 01:15:26 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Dave Page Date: Mon, 18 Jan 2021 09:15:15 +0000 Message-ID: Subject: Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1 To: Khushboo Vashi Cc: Akshay Joshi , Aditya Toshniwal , pgadmin-hackers Content-Type: multipart/alternative; boundary="0000000000009c5e8805b929292b" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --0000000000009c5e8805b929292b Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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. > - krb5 libs are not bundled with the Desktop packages, so added the gssap= i > 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 Vash= i) > 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 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 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 o= n >>> 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 tha= t 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 adde= d >>>> 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 installatio= n >>>> 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 look= s >>>>>>>>>> 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 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 throu= gh a browser >>>>>>>>>>>> which will bypass the login page entirely if the Kerberos Auth= entication >>>>>>>>>>>> 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 #582= 9 >>>>>>>>>>>> >>>>>>>>>>>> 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 >> >> --=20 Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EDB: http://www.enterprisedb.com --0000000000009c5e8805b929292b Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi

On Mon, Jan 18, 2021 at 7:30 AM Khushbo= o Vashi <khushboo.vas= hi@enterprisedb.com> wrote:
Hi,

Please find the = attached updated patch with the below changes:

- D= ependencies are added into Linux packages in the RPM/DEBs.
-= Dev packages are added in the setup scripts for Linux.
- The req= uired packages are added in the Dockerfile.
- Conditional gss= api 1.6.2 dependency is added for Python 3.5 in requirements.txt.

1.6.9 is the last release that sup= ports Python 3.4+. We should use that rather than older versions.
=C2=A0
- krb5 libs are not bundled with the Desktop packages, so ad= ded the gssapi dependency into the try/catch block.=C2=A0
- = .dockerignore is introduced to ignore unwanted files/folders like node_modu= les etc., which will make the docker build fast.=C2=A0(By Ashesh=C2=A0Vashi= )

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

=C2=A0

<= div>Thanks,
Khushboo

On Fri, Jan 15, 2021 at 3:48 PM D= ave Page <dpage@p= gadmin.org> wrote:
And another thought...

Some = of the Jenkins QA jobs setup the virtual environment for running tests them= selves. 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 Th= u, Jan 14, 2021 at 4:34 PM Dave Page <dpage@pgadmin.org> wrote:
FYI, I did a quick t= est (and browse of PyPI):

- On Windows, it seems there i= s a binary wheel available:

(gssapi) C:\Users\dpag= e>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
Col= lecting decorator
=C2=A0 Downloading decorator-4.4.2-py2.py3-none-any.wh= l (9.2 kB)
Installing collected packages: decorator, gssapi
Successfu= lly 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 ad= ditional binary dependencies.

This should simplify= things a lot - we just need to ensure the build scripts use the binary pac= kage on Windows, and install the build deps on the Linux/Docker environment= s=C2=A0(and update the package builds with the additional dependencies of c= ourse).


On Thu, Jan 14, 2021 at 4:04 PM Dave Page <= ;dpage@pgadmin.org> wrote:
Hi Khushboo,

As you know, this has been r= olled back as the buildfarm blew up. I think there are a number of TODOs th= at need to be addressed, given that the gssapi Python module is dependent o= n 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 tha= t describe how to build packages will need to be updated.
- The D= ockerfile will need to be modified to add the required packages.
= - The Windows build will need to be updated so the installer ships addition= al required DLLs.
- Are there any additional macOS dependencies? = If so, they need to be handled.

In the buildfarm:<= /div>

- 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 ou= rselves to ensure we're consistently using the same buildchain. If we d= o build it ourselves:
=C2=A0 - Will the Python package find it du= ring it's build?
=C2=A0 - We'll need to create a Jenkins = job to perform the build.
- Is any work required on macOS, or doe= s it ship with everything that's needed? If not, we'll need to buil= d it, and create the Jenkins job.

One final though= t: on Windows/macOS, can we force a binary installation from PIP (pip insta= ll --only-binary=3Dgssapi gssapi)? If so, will that include the required li= braries, as psycopg2-binary=C2=A0does?


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

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

Please= ignore my previous patch, attached the updated one.

Thanks,
Khushboo

On Thu, Jan 14, 2021 at 12:17 PM Khushbo= o 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= <aks= hay.joshi@enterprisedb.com> wrote:
Hi=C2=A0Khushboo

<= div>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@enterpris= edb.com> wrote:
Hi,

Please find= the attached updated patch.

Thanks,
Khu= shboo

On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal <aditya.toshniwal= @enterprisedb.com> wrote:
Hi Khushboo,

I've just done the co= de 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= =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



--


--


--


--
--0000000000009c5e8805b929292b--