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

On Mon, Jan 18, 2021 at 9:37 AM Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

>
>
> On Mon, Jan 18, 2021 at 2:45 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> 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.
>>
> As per the https://pypi.org/project/gssapi/*1.6.9*/, it says Requires: Python
> >=3.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 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
>>
>>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: http://www.enterprisedb.com