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

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*
>>
>