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

Поиск
Список
Период
Сортировка
От Aditya Toshniwal
Тема Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1
Дата
Msg-id CAM9w-_koVr9Qy3hDaSaDC9pX6jeDzj5gSghYjT2yxQ2h6zbr=w@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1  (Akshay Joshi <akshay.joshi@enterprisedb.com>)
Ответы Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1  (Khushboo Vashi <khushboo.vashi@enterprisedb.com>)
Список pgadmin-hackers
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.

+@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.joshi@enterprisedb.com> wrote:
Hi Aditya

Can you please do the code review?

On Tue, Dec 22, 2020 at 3:44 PM Khushboo Vashi <khushboo.vashi@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
Mobile: +91 976-788-8246



--
Thanks,
Aditya Toshniwal
pgAdmin hacker | Sr. Software Engineer | edbpostgres.com
"Don't Complain about Heat, Plant a TREE"

В списке pgadmin-hackers по дате отправления:

Предыдущее
От: Aditya Toshniwal
Дата:
Сообщение: Re: [pgAdmin][RM5571] Expression in exclusion constraint is misinterpreted and quoted as column name by mistake
Следующее
От: Aditya Toshniwal
Дата:
Сообщение: [pgAdmin][RM5282] "Count Rows" option missing from partition sub tables