On Mon, Jan 18, 2021 at 2:45 PM Dave Page <dp...@pgadmin.org> wrote:
> Hi
>
> On Mon, Jan 18, 2021 at 7:30 AM Khushboo Vashi <
> khushboo.va...@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: Python
>=3.6.*
>
>
>> - 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 <dp...@pgadmin.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 <dp...@pgadmin.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 <dp...@pgadmin.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.jo...@enterprisedb.com> wrote:
>>>>>
>>>>>> Thanks, patch applied.
>>>>>>
>>>>>> On Thu, Jan 14, 2021 at 1:42 PM Khushboo Vashi <
>>>>>> khushboo.va...@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.va...@enterprisedb.com> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Please find the attached updated patch.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Khushboo
>>>>>>>>
>>>>>>>> On Thu, Jan 14, 2021 at 12:00 PM Akshay Joshi <
>>>>>>>> akshay.jo...@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.va...@enterprisedb.com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Please find the attached updated patch.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Khushboo
>>>>>>>>>>
>>>>>>>>>> On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal <
>>>>>>>>>> aditya.toshni...@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.jo...@enterprisedb.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi Aditya
>>>>>>>>>>>>
>>>>>>>>>>>> Can you please do the code review?
>>>>>>>>>>>>
>>>>>>>>>>>> On Tue, Dec 22, 2020 at 3:44 PM Khushboo Vashi <
>>>>>>>>>>>> khushboo.va...@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 <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
>
>
