Re: Patch: Two-factor Authentication (RM #6543)
От | Akshay Joshi |
---|---|
Тема | Re: Patch: Two-factor Authentication (RM #6543) |
Дата | |
Msg-id | CANxoLDcH46q6YEjd_dWD23Mk43+SAaV2ayk=MxY+o7-RRxx4sQ@mail.gmail.com обсуждение исходный текст |
Ответы |
Re: Patch: Two-factor Authentication (RM #6543)
|
Список | pgadmin-hackers |
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.
Obviously - it will only be available for server mode.
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB PostgresMobile: +91 976-788-8246
Вложения
В списке pgadmin-hackers по дате отправления: