public inbox for [email protected]
help / color / mirror / Atom feedFrom: Akshay Joshi <[email protected]>
To: Khushboo Vashi <[email protected]>
Cc: Aditya Toshniwal <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1
Date: Thu, 14 Jan 2021 12:00:25 +0530
Message-ID: <CANxoLDcXZnR6L3QMRoBDPqkm_Qaqvj6OYshk4anZ6DMGfk8UJQ@mail.gmail.com> (raw)
In-Reply-To: <CAFOhELeFZAXSWQYFoRGvfOEZ+Kt_sWWxPFte17o-maqq0JshXg@mail.gmail.com>
References: <CAFOhELdXhWMR2zS4dnH+SudN0s7LiENH+vczC0YhuifPgm+G5g@mail.gmail.com>
<CANxoLDeKWqKP-6=KRTY_-vDSZv2g=P7dKJkzWPS7aOB3EoZOoA@mail.gmail.com>
<CAM9w-_koVr9Qy3hDaSaDC9pX6jeDzj5gSghYjT2yxQ2h6zbr=w@mail.gmail.com>
<CAFOhELeFZAXSWQYFoRGvfOEZ+Kt_sWWxPFte17o-maqq0JshXg@mail.gmail.com>
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 <
[email protected]> wrote:
> Hi,
>
> Please find the attached updated patch.
>
> Thanks,
> Khushboo
>
> On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal <
> [email protected]> 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.
>
>
>> [email protected]("/kerberos_login",
>>
>> + endpoint="kerberos_login", methods=["GET"])
>>
>>
>> [email protected]("/kerberos_logout",
>>
>> + endpoint="kerberos_logout", methods=["GET"])
>>
>>
>>
>>
>
>> On Tue, Dec 22, 2020 at 6:07 PM Akshay Joshi <
>> [email protected]> wrote:
>>
>>> Hi Aditya
>>>
>>> Can you please do the code review?
>>>
>>> On Tue, Dec 22, 2020 at 3:44 PM Khushboo Vashi <
>>> [email protected]> 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*
view thread (32+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1
In-Reply-To: <CANxoLDcXZnR6L3QMRoBDPqkm_Qaqvj6OYshk4anZ6DMGfk8UJQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox