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*
