Re: Patch: Two-factor Authentication (RM #6543)

Thanks, the patch applied.

On Wed, Dec 1, 2021 at 11:14 PM Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
wrote:

> On Wed, Sep 29, 2021 at 1:29 PM Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> Hi Ashesh
>>
>> I am sorry but have to revert this patch as it causes build failures on
>> OSX and docker. Will commit once issue is resolved.
>>
> Hi Team,
>
> Please find the updated patch with the necessary changes for OSX and
> docker build failure.
>
>>
>> On Thu, Sep 23, 2021 at 1:36 PM Ashesh Vashi <
>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> On Tue, Aug 31, 2021 at 1:03 PM Akshay Joshi <
>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Ashesh
>>>>
>>> Hi Team,
>>>
>>> Please find the updated patch with the following review comments, and
>>> also some fixes related to validating the code on email
>>> authentication,found during unit testing.
>>>
>>>>
>>>> Following are the review comments:
>>>>
>>>> - *GUI*:
>>>> - The two-factor authentication dialog's size is too big, can we
>>>> make it a bit small?
>>>>
>>>> As discussed in the review meeting, let's keep it as is.
>>> I've modified the backgrid class to work better with smaller resolution
>>> too.
>>>
>>>>
>>>> - Dialog's header is showing "AlertifyJS", please provide some
>>>> valid header name.
>>>>
>>>> Done
>>>
>>>>
>>>> - 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.
>>>>
>>>> I will work with Nidhi to add more robust documentation.
>>>
>>>>
>>>> - 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.
>>>>
>>>> As discussed in the review meeting, we won't implement that.
>>> You can create a RM in case you're keen to work on it in future.
>>>
>>>>
>>>> - *Code*:
>>>> - Copyright headers are missing in some of the new files.
>>>>
>>>> Done.
>>>
>>>>
>>>> - Add comments/function descriptions wherever needed. It will be
>>>> easy for others to understand the logic.
>>>>
>>>> Done.
>>>
>>>>
>>>> - Remove unused imports from the new files.
>>>>
>>>> Done.
>>>
>>>>
>>>> - Sonarlint showing warning "Replace <i> tag with <em>" for some
>>>> HTML files.
>>>>
>>>> We always use <i> for icons. It's not an issue, you can ignore it.
>>>
>>>>
>>>> - *send_email_otp.html* and *send_email_otp.txt* both files having
>>>> the same content, they both are needed?
>>>>
>>>> Both are needed, as it depends on EMAIL_PLAINTEXT and EMAIL_HTML
>>> settings of the flask_security package, which is used to send the email.
>>>
>>>>
>>>> - In "*test_mfa_view.py*" file line no 26 showing "Unresolved
>>>> reference ValidationException"
>>>>
>>>> Done
>>>
>>>>
>>>> - In the "*mfa/tests/utils.py"* file Remove unused class
>>>> declaration "class DummyAuth". Also, update the "return true" with "return
>>>> True" at line no 31th
>>>>
>>>> That's necessary - that is added for a reason for adding unit test
>>> cases for defining a auth class for testing purpose.
>>> An object of the class is initialized automatically by the registry, so
>>> - it is required. I've add # NOSONAR to suppression the warning.
>>>
>>>>
>>>> *Should* *we consider*: What if pgAdmin 4 Administrator wants MFA
>>>> force registration for all the other users except himself/herself?
>>>>
>>> It is out of scope of this patch.
>>>
>>> -- Thanks, Ashesh
>>>
>>>>
>>>> *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(dot)vashi(at)enterprisedb(dot)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(dot)vashi(at)enterprisedb(dot)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.
>>>>>> [image: 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.enterprisedb.com>
>>>>>>
>>>>>>
>>>>>> *http://www.linkedin.com/in/asheshvashi*
>>>>>> <http://www.linkedin.com/in/asheshvashi>
>>>>>>
>>>>>
>>>>
>>>> --
>>>> *Thanks & Regards*
>>>> *Akshay Joshi*
>>>> *pgAdmin Hacker | Principal Software Architect*
>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>
>>>> *Mobile: +91 976-788-8246*
>>>>
>>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>> *pgAdmin Hacker | Principal Software Architect*
>> *EDB Postgres <http://edbpostgres.com>*
>>
>> *Mobile: +91 976-788-8246*
>>
>

--
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres <http://edbpostgres.com>*

*Mobile: +91 976-788-8246*