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*
>
RM_5457_v2.patch
Description: Binary data
