From: | Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com> |
---|---|
To: | Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com> |
Cc: | pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org> |
Subject: | Re: [pgAdmin][Patch] - RM #5940 - Add support for Oauth 2 authentication |
Date: | 2021-07-05 12:09:12 |
Message-ID: | CANxoLDcfbSck_siAM+kaEQnedwZURPLP_5W8krcpwnfzA6bULQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
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>.
- 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.
-
*Code Review:*
- Empty "__init__.py" file is added in the web folder, I think that is
not required.
- Any reason to change *messages.pot* file manually? It will be updated
when we update the message catalog.
- Fixed SonarQube issues in the new file 'oauth2.py' and
'test_oauth2_with_mocking.py'. Remove unused imports from both files.
- Add copyright header to "kerberos.js" file.
*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?
*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*
From | Date | Subject | |
---|---|---|---|
Next Message | Yogesh Mahajan | 2021-07-05 15:09:34 | [pgAdmin][Patch] - Housekeeping #6582 - [React] Port Extension object to react |
Previous Message | Dave Page | 2021-07-05 08:51:17 | Re: [patch][pgAdmin] Fix for pgadmin4-linux-qa #1651 failure |