Postgres Pro
Downloads
Home   >   mailing lists

Re: Patch: Two-factor Authentication (RM #6543) - Mailing list pgadmin-hackers

From Akshay Joshi
Subject Re: Patch: Two-factor Authentication (RM #6543)
Date
Msg-id CANxoLDcH46q6YEjd_dWD23Mk43+SAaV2ayk=MxY+o7-RRxx4sQ@mail.gmail.com
Whole thread Raw
Responses Re: Patch: Two-factor Authentication (RM #6543)  (Ashesh Vashi <ashesh.vashi@enterprisedb.com>)
List pgadmin-hackers
Tree view
Hi Ashesh 

Following are the review comments:
  • GUI:
    • The two-factor authentication dialog's size is too big, can we make it a bit small? 
    • Dialog's header is showing "AlertifyJS", please provide some valid header name.
    • Documentation: Let users know from where they can access the "Two-Factor Authentication" dialog, the screenshot of the top-right menu option, and few lines.
    • Do we need MFA_ENABLED and MFA_FORCE_REGISTRATION two different parameters? For the users, if MFA_ENABLED is set to True then it is as good as users bound to register for MFA and after the successful validation, they can access the pgAdmin 4 webpage.
  • Code:
    • Copyright headers are missing in some of the new files.
    • Add comments/function descriptions wherever needed. It will be easy for others to understand the logic.
    • Remove unused imports from the new files.
    • Sonarlint showing warning "Replace <i> tag with <em>" for some HTML files.
    • send_email_otp.html and send_email_otp.txt both files having the same content, they both are needed? 
    • In "test_mfa_view.py" file line no 26 showing "Unresolved reference ValidationException"
    • In the "mfa/tests/utils.py" file Remove unused class declaration "class DummyAuth". Also, update the "return true" with "return True" at line no 31

Should we consider: What if pgAdmin 4 Administrator wants MFA force registration for all the other users except himself/herself? 

Note: I haven't run API test cases. Checked PEP8, Linter, and Jasmine tests.

On Mon, Aug 30, 2021 at 6:23 PM Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Hi Team,

Please find the update patch after fixing some issues introduced during refactoring.

On Mon, Aug 30, 2021 at 11:48 AM Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Hi Team,

Please find the attached patch for the two-factor authentication.
It supports sending an OTP to email, or use google authenticator to increase the authentication process.

As discussed earlier, I have followed the following design for authentication.
Screenshot 2021-06-17 at 7.09.47 PM.png


Obviously - it will only be available for server mode.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


http://www.linkedin.com/in/asheshvashi



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246

Attachment

pgadmin-hackers by date:

Previous
From: Khushboo Vashi
Date:
Subject: Re: [pgAdmin][PATCH] Add OAUTH2_SCOPE variable for scope configuration
Next
From: Nico Rikken
Date:
Subject: Re: [pgAdmin][PATCH] Add OAUTH2_SCOPE variable for scope configuration