public inbox for [email protected]
help / color / mirror / Atom feedFrom: Akshay Joshi <[email protected]>
To: Khushboo Vashi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin][Patch] - RM #5940 - Add support for Oauth 2 authentication
Date: Tue, 6 Jul 2021 13:25:05 +0530
Message-ID: <CANxoLDf=91g332-KReFRzc=G_X84cA3qjD3wrurLwRXAQdi0Bw@mail.gmail.com> (raw)
In-Reply-To: <CAFOhELdWYizdZYt1BZFeVNYt2_gd3ZWCZP6NfC_fbhpA5Zd2bQ@mail.gmail.com>
References: <CAFOhELek51f+q6gb2Sdx4CX94fSNVfye2k8-dXKJm3afDWiL6w@mail.gmail.com>
<CAFOhELdfoR+d16-K2dcr11JaXz9eqgksg+1Z1X0Qgq-BZ6AqbQ@mail.gmail.com>
<CANxoLDcfbSck_siAM+kaEQnedwZURPLP_5W8krcpwnfzA6bULQ@mail.gmail.com>
<CAFOhELdWYizdZYt1BZFeVNYt2_gd3ZWCZP6NfC_fbhpA5Zd2bQ@mail.gmail.com>
Thanks, the patch applied.
On Tue, Jul 6, 2021 at 12:01 PM Khushboo Vashi <
[email protected]> wrote:
> Hi Akshay,
>
> Please find the attached updated patch.
>
> Thanks,
> Khushboo
> On Mon, Jul 5, 2021 at 5:39 PM Akshay Joshi <[email protected]>
> 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 <
>> [email protected]> wrote:
>>
>>> Hi,
>>>
>>> Please find the attached rebased patch.
>>>
>>> Thanks,
>>> Khushboo
>>>
>>> On Thu, Jun 17, 2021 at 4:22 PM Khushboo Vashi <
>>> [email protected]> 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*
>>
>
--
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres <http://edbpostgres.com>*
*Mobile: +91 976-788-8246*
view thread (5+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: [pgAdmin][Patch] - RM #5940 - Add support for Oauth 2 authentication
In-Reply-To: <CANxoLDf=91g332-KReFRzc=G_X84cA3qjD3wrurLwRXAQdi0Bw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox