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 1l1RGi-0002ts-06 for pgadmin-hackers@arkaria.postgresql.org; Mon, 18 Jan 2021 09:56:16 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1l1RGg-0006OK-V6 for pgadmin-hackers@arkaria.postgresql.org; Mon, 18 Jan 2021 09:56:14 +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 1l1RGg-0006OC-CV for pgadmin-hackers@lists.postgresql.org; Mon, 18 Jan 2021 09:56:14 +0000 Received: from mail-ej1-x62b.google.com ([2a00:1450:4864:20::62b]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1l1RGb-0005st-T3 for pgadmin-hackers@postgresql.org; Mon, 18 Jan 2021 09:56:12 +0000 Received: by mail-ej1-x62b.google.com with SMTP id q22so22813528eja.2 for ; Mon, 18 Jan 2021 01:56:09 -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=TlZzBqueuL1IxB3PbOmXXOJB71FwV9FQ5AAHVnAsTRk=; b=cPyEkm/hHH8eVOzWALjd2a2hObaTLlauHeMMrGzA+CJy6sfn14IGajpf+4j0iek6an IwzhqYlgiIYuHOZODI3gVt/in8y6Kt5W+dEVQWun/goHx0AnOvkNhKnZ2Rs9ihPK/X0z VizybsA1fnbEyh38Ea0PCjUvQ4ARvAQruQC1dmKUvI3DeNXpGO9lz/hdPBcM//qm2WVp X+y9kDUUxZBZ5Ao6Rm1rge6VrSdceco4Uok+HPL7fWLtd4mDBY5QQG6f7gdSpCZYV5ac LT6ex6zIZ+XIIoiXg4cOVkhiUL5642pmKvYiWdoZ0DYkefCYAbSRXDd/0kw9+5tXMdYb NkKA== 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=TlZzBqueuL1IxB3PbOmXXOJB71FwV9FQ5AAHVnAsTRk=; b=iwJmtvt6iFyKY09ZdU6vLZSyAY30gLR3SweTqA7JQktXn+X9Qb/8JfNlQRSKVs3F6i YP+nYCoY3gNEWatNVaC0fcdVbdAiQXiaHfPQhUeTMI2ztpRpaxFtI1J9aehAnwKdKx/W HKwG+BB0QE6Unbfjyzi7hnYJS3+PaWCgmvW5+vPzJ/xOJ64SXg6bKhmo+CMqPM7zTJ32 GiG+P/oLwBlMedGnpNYQKI+U9UFrdaigjJz8dJ37rTTTbmRr4Wf9CC90E6U7JmYTYeUT CLBwklT9H0q9f/1V872RUG8JUyeafD1Ta7XsgrLf7d2rbon2gOKpDzZ/s1iMSiGRE8Yh a3sg== X-Gm-Message-State: AOAM5302MjVsv+lEjD0UvueySXOGb6TxpldurCTAJ1z/luyYyJm10O+D lq4KYvCQJm7+KZmbkE6Vl6M8rGBX3/s5k1lnzXEGmQ== X-Google-Smtp-Source: ABdhPJxKjJDbd0+UGHSzg8hMT9tXJgJiUAXpEO71n8lXwW+xiXnj/XZDffmiHhlMHVZckkMwcxEoxyT/loj5UKNOHtU= X-Received: by 2002:a17:906:944a:: with SMTP id z10mr823228ejx.96.1610963767055; Mon, 18 Jan 2021 01:56:07 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Dave Page Date: Mon, 18 Jan 2021 09:55:55 +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="00000000000014f82805b929bb02" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --00000000000014f82805b929bb02 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, Jan 18, 2021 at 9:37 AM Khushboo Vashi < khushboo.vashi@enterprisedb.com> wrote: > > > 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: Py= thon > >=3D3.6.* > I think that's the metadata for the latest package version on the left. If you read the main text, it says: Requirements Basic - A working implementation of GSSAPI (such as from MIT Kerberos) which includes header files - a C compiler (such as GCC) - either the enum34 Python package or Python 3.4+ - the six and decorator python package For 1.6.10, that changed to: Requirements Basic - A working implementation of GSSAPI (such as from MIT Kerberos) which supports delegation and includes header files - a C compiler (such as GCC) - Python 3.6+ (older releases support older versions, but are unsupported) - the decorator python package > >> >> >>> - 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 Va= shi) >>> >> >> 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 Window= s. >>>> >>>> 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 an= y >>>>> 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 t= hat 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 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 b= e >>>>>> 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 Jenkin= s job. >>>>>> >>>>>> One final thought: on Windows/macOS, can we force a binary >>>>>> installation from PIP (pip install --only-binary=3Dgssapi gssapi)? I= f 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 rephrasin= g 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["GE= T"]) >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> 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 thr= ough a browser >>>>>>>>>>>>>> which will bypass the login page entirely if the Kerberos Au= thentication >>>>>>>>>>>>>> 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 >>>>>> >>>>>> >>>>> >>>>> -- >>>>> 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 --00000000000014f82805b929bb02 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Mon, Jan 18, 2021 at 9:37 AM Khush= boo Vashi <khushboo.v= ashi@enterprisedb.com> wrote:


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

=
On Mon, Ja= n 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 add= ed for Python 3.5 in requirements.txt.
<= br>
1.6.9 is the last release that supports Python 3.4+. We shoul= d use that rather than older versions.
A= s per the=C2=A0https://pypi.org/project/gssapi/1.6.9/, it says=C2=A0Requires:=C2=A0Python >=3D3.6.*

I think that's the meta= data for the latest package version on the left. If you read the main text,= it says:

Re= quirements

Basic

  • A working implementation of GSSAPI= (such as from MIT Kerberos) which includes header files
  • a C compiler (such as GCC)
  • either the=C2=A0enum34=C2=A0Python package or Python 3.4+
  • the=C2=A0six=C2=A0and=C2=A0dec= orator=C2=A0python package
  • =C2=A0
    For 1.6.10, that changed to:

    Requirements

    =

    Basic

    • A working imp= lementation of GSSAPI (such as from MIT Kerberos) which supports delegation= and includes header files
    • a C compiler (such as GCC)
    • Python 3.6+ (older releases support older versions, but are unsup= ported)
    • the=C2=A0decorator=C2=A0python package

    =C2=A0
    =C2=A0
    =
    - krb5 libs are not bundled with the Desktop pac= kages, so added the gssapi dependency into the try/catch block.=C2=A0
    =
    - .dockerignore is introduced to ignore unwanted files/folders li= ke node_modules etc., which will make the docker build fast.=C2=A0(By Ashes= h=C2=A0Vashi)

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

    =C2=A0

    Thanks,
    Khushboo

    On Fri, Jan 15, 2021= at 3:48 PM Dave Page <dpage@pgadmin.org> wrote:
    And another thought...

    Some of the Jenkins QA jobs setup the virtual environment for runnin= g 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 d= id a quick test (and browse of PyPI):

    - On Windows, it s= eems there is a binary wheel available:

    (gssapi) C= :\Users\dpage>pip install gssapi
    Collecting gssapi
    =C2=A0 Download= ing 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 Downloading decorator-4.4.2-py2.py3= -none-any.whl (9.2 kB)
    Installing collected packages: decorator, gssapi<= br>Successfully installed decorator-4.4.2 gssapi-1.6.12

    <= /div>
    - On macOS, the wheel is built by pip, but it doesn't seem to= have any additional binary dependencies.

    This sho= uld simplify things a lot - we just need to ensure the build scripts use th= e binary package on Windows, and install the build deps on the Linux/Docker= environments=C2=A0(and update the package builds with the additional depen= dencies of course).


    On Thu, Jan 14, 2021 at 4:04 PM Dav= e Page <dpage@pga= dmin.org> wrote:
    Hi Khushboo,

    As you know, this = has been rolled back as the buildfarm blew up. I think there are a number o= f TODOs that need to be addressed, given that the gssapi Python module is d= ependent 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 nee= d to have the -dev packages added as appropriate.
    - The various R= EADMEs that describe how to build packages will need to be updated.
    - The Dockerfile will need to be modified to add the required packages.<= /div>
    - The Windows build will need to be updated so the installer ship= s additional required DLLs.
    - Are there any additional macOS depe= ndencies? If so, they need to be handled.

    In the b= uildfarm:

    - All Linux build VMs need to be updated= with the additional dependencies.
    - On Windows, we need to figur= e out how to build/ship KfW. It's a pain to build, which we would typic= ally do ourselves to ensure we're consistently using the same buildchai= n. 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 Jenkins job to perform the build.
    - Is any work required on mac= OS, or does it ship with everything that's needed? If not, we'll ne= ed to build it, and create the Jenkins job.

    One fi= nal thought: on Windows/macOS, can we force a binary installation from PIP = (pip install --only-binary=3Dgssapi gssapi)? If so, will that include the r= equired libraries, as psycopg2-binary=C2=A0does?

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

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

    <= div>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 Aks= hay Joshi <akshay.joshi@enterprisedb.com> wrote:
    Hi=C2=A0Khushboo
    Seems you have attached the wrong patch. Please send the updat= ed patch.

    On Wed, Jan 13, 2021 at 2:35 PM Khushboo Vashi <khushboo.vash= i@enterprisedb.com> wrote:
    Hi,

    = Please find the attached updated patch.

    Thanks,
    Khushboo

    On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal <<= a href=3D"mailto:aditya.toshniwal@enterprisedb.com" target=3D"_blank">adity= a.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 constant= s 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



    --


    --


    --


    --


    --
    --00000000000014f82805b929bb02--