Re: [pgAdmin][Patch] - RM #5940 - Add support for Oauth 2 authentication

Hi Akshay,

Please find the attached updated patch.

Thanks,
Khushboo
On Mon, Jul 5, 2021 at 5:39 PM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

> Hi Khushboo
>
> Following are the review comments:
>
> *Scenario *: AUTHENTICATION_SOURCES = ['oauth2', 'internal']
>
> - Don't set the OAUTH2_DISPLAY_NAME, Login button shows 'Login with
> None' which should be change. Maybe placeholder like <oauth2 display name>.
>
> Fixed

>
> -
> - Enter the email address and password. Click on the "Login with ..."
> button, internal server error displayed on the page, we should throw a
> proper error message.
>
> Fixed

>
> -
>
> *Code Review:*
>
> - Empty "__init__.py" file is added in the web folder, I think that is
> not required.
>
> By mistake, it was added.

>
> - Any reason to change *messages.pot* file manually? It will be
> updated when we update the message catalog.
>
> I must have run make docs, removed.

>
> - Fixed SonarQube issues in the new file 'oauth2.py' and
> 'test_oauth2_with_mocking.py'. Remove unused imports from both files.
>
> Fixed

>
> - Add copyright header to "kerberos.js" file.
>
> Fixed

>
> *Documentation*:
>
> - Remove "of the" from the description of OAUTH2_NAME it is duplicated.
> - String "Authorisation" should be "Authorization".
> - Instead of providing "Oauth2 secret", "Oauth2 base url" .... can we
> provide some more detailed information?
>
>
> Fixed.

> *Note*: I haven't tested the patch with the proper OAuth2 configuration.
>
> On Mon, Jun 28, 2021 at 4:52 PM Khushboo Vashi <
> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>> Hi,
>>
>> Please find the attached rebased patch.
>>
>> Thanks,
>> Khushboo
>>
>> On Thu, Jun 17, 2021 at 4:22 PM Khushboo Vashi <
>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi,
>>>
>>> Please find the attached patch for the RM #5940 - Add support for Oauth
>>> 2 authentication.
>>>
>>> The patch includes:
>>>
>>> - Pluggable Oauth 2 authentication support with the multiple providers
>>> - Configurable support for Master Key in Oauth2 and Kerberos
>>> authentication, so user can save the database server password
>>> - Introduced the separate module/blueprint for kerberos and Oauth2 under
>>> authentication module for the readability and maintainability
>>>
>>> Note: The initial patch for Oauth2 authentication was sent by Florian
>>> Sabonchi, I have worked on top of it.
>>>
>>>
>>> Thanks,
>>> Khushboo
>>>
>>>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
> *pgAdmin Hacker | Principal Software Architect*
> *EDB Postgres <http://edbpostgres.com>*
>
> *Mobile: +91 976-788-8246*
>