Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Hi

On Mon, Jan 18, 2021 at 7:30 AM Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)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 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 Vashi)
>

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 <dpage(at)pgadmin(dot)org> 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 <dpage(at)pgadmin(dot)org> 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)
>>> |████████████████████████████████| 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 <dpage(at)pgadmin(dot)org> 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
>>>> 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 <
>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Thanks, patch applied.
>>>>>
>>>>> On Thu, Jan 14, 2021 at 1:42 PM Khushboo Vashi <
>>>>> khushboo(dot)vashi(at)enterprisedb(dot)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(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Please find the attached updated patch.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Khushboo
>>>>>>>
>>>>>>> On Thu, Jan 14, 2021 at 12:00 PM Akshay Joshi <
>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)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(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Please find the attached updated patch.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Khushboo
>>>>>>>>>
>>>>>>>>> On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal <
>>>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)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.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> +(at)blueprint(dot)route("/kerberos_login",
>>>>>>>>>>
>>>>>>>>>> + endpoint="kerberos_login", methods=["GET"])
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> +(at)blueprint(dot)route("/kerberos_logout",
>>>>>>>>>>
>>>>>>>>>> + endpoint="kerberos_logout", methods=["GET"])
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> On Tue, Dec 22, 2020 at 6:07 PM Akshay Joshi <
>>>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Aditya
>>>>>>>>>>>
>>>>>>>>>>> Can you please do the code review?
>>>>>>>>>>>
>>>>>>>>>>> On Tue, Dec 22, 2020 at 3:44 PM Khushboo Vashi <
>>>>>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)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 <http://edbpostgres.com>*
>>>>>>>>>>>
>>>>>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> Thanks,
>>>>>>>>>> Aditya Toshniwal
>>>>>>>>>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
>>>>>>>>>> <http://edbpostgres.com>
>>>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> *Thanks & Regards*
>>>>>>>> *Akshay Joshi*
>>>>>>>> *pgAdmin Hacker | Principal Software Architect*
>>>>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>>>>
>>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>>
>>>>>>>
>>>>>
>>>>> --
>>>>> *Thanks & Regards*
>>>>> *Akshay Joshi*
>>>>> *pgAdmin Hacker | Principal Software Architect*
>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>
>>>>> *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