public inbox for [email protected]
help / color / mirror / Atom feedFrom: Aditya Toshniwal <[email protected]>
To: Khushboo Vashi <[email protected]>
Cc: Akshay Joshi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1
Date: Fri, 1 Jan 2021 13:07:21 +0530
Message-ID: <CAM9w-_koVr9Qy3hDaSaDC9pX6jeDzj5gSghYjT2yxQ2h6zbr=w@mail.gmail.com> (raw)
In-Reply-To: <CANxoLDeKWqKP-6=KRTY_-vDSZv2g=P7dKJkzWPS7aOB3EoZOoA@mail.gmail.com>
References: <CAFOhELdXhWMR2zS4dnH+SudN0s7LiENH+vczC0YhuifPgm+G5g@mail.gmail.com>
<CANxoLDeKWqKP-6=KRTY_-vDSZv2g=P7dKJkzWPS7aOB3EoZOoA@mail.gmail.com>
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'
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
+
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
4) KERBEROSAuthentication could be KerberosAuthentication
class KERBEROSAuthentication(BaseAuthentication):
5) You can use the constants (ldap, kerberos) you had defined when creating
a user.
+ 'auth_source': 'kerberos'
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. Also, even though the
method GET works, we should use the POST method for login and DELETE for
logout.
[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"
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: <CAM9w-_koVr9Qy3hDaSaDC9pX6jeDzj5gSghYjT2yxQ2h6zbr=w@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