Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1m0fvO-0000X3-2K for pgadmin-hackers@arkaria.postgresql.org; Tue, 06 Jul 2021 07:55:22 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1m0fvN-0006qO-2G for pgadmin-hackers@arkaria.postgresql.org; Tue, 06 Jul 2021 07:55:21 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1m0fvM-0006q8-Ne for pgadmin-hackers@lists.postgresql.org; Tue, 06 Jul 2021 07:55:20 +0000 Received: from mail-io1-xd33.google.com ([2607:f8b0:4864:20::d33]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1m0fvK-0003yy-0R for pgadmin-hackers@postgresql.org; Tue, 06 Jul 2021 07:55:20 +0000 Received: by mail-io1-xd33.google.com with SMTP id u7so21484429ion.3 for ; Tue, 06 Jul 2021 00:55:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=bSB95ChK08NkMnm8eOrxDsBkEWmV7PZLMA73mYqHBkI=; b=a4V2VqcYOr5kov+EvSNHsv+DN4aicksCjYHjNua2c2rDG7KTSOHMaHn4yEe4OCYzJx e8emV5tLq5sOc6xDpZ4wQk4Zcj4F+wFBlfbNW49EtGRJ0rAFx2FNEG/DZW6ngUOSsBK+ Pucg97ozRmHLWCE6smqno+wzv6rVz5M84wTKo+bMbpj1fUb2z6QpxOFlP0mvlDdnwhM+ OO7hlUGTOrU8Z3uaLXLOXJ0sgeNOaceTsEfJgeo4rsirF91VvAmsddtESAP8+FnUMXhf aH8dkkwBmDvy3zAxAHgIYehQZ3MytByLzmah7AAXMwEJrBswsJf7z9HII7cldj6WgmbP 7KBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=bSB95ChK08NkMnm8eOrxDsBkEWmV7PZLMA73mYqHBkI=; b=JYb+cp0w0pE08ZYgKJq5UOc4lMaVPjJ7vI4mWGncq3iSCuf2lMlznbeRq6arJ2UqAx Mq2wA2IqLB0uiPKQ54ZmiF9sIEgdVrlwLwe8GRxZQZdPGp6KKhO5C/xr3elSJrUo04pr ycVgpU4inN6oF5TfFe72Rq+gjPUM1tX5TvC2bgTX3BrSnHiJ/8fDpaaXF26/4+9txx0Q MFQHSIuJv03BcPGl93U6JU+8AHRBt8+Ps6DYcL1L4uE3FvybNMigtmtejsaY/YpUJ2LN 8OD49k3IvbVsdmP2xIUDKVHt+5ZIhZPBEu9pzRaa5yxR3+dLo+9L5y/2D7vHh3+a26WJ HQTQ== X-Gm-Message-State: AOAM5336CKk2fSL6RINYe+y1opL/boev9bg49Qs6Uxqzwf/cb4FdVTCK lJuFZ+oueMrpRDkPt6aLcKIhWhwJo8r3ch36HKoh36UzNU+7yzBx6qcsX/p8R6Fr1mUgxX13bsP ykforQjw5CIUYO/5T6Y76GEJYuuDk6dJqNEBF9XKJja1MPaW67PuhOwI7KOupgLpDZDj2yJ3zxQ A3lx2t6uNoxFGiYUzcJr2hIXHBvkW0lZ/gi1zUpLhBGxrSOP0AEAU9duOUMA== X-Google-Smtp-Source: ABdhPJz2obJlIIjWH+90VDx1I7+khQXg9AHyGh/OfJ7FbKp7kTp+p89WzI5JxHgpAbffAJnFe/LQl72rpkbcpEenODo= X-Received: by 2002:a5d:80da:: with SMTP id h26mr5056838ior.206.1625558116385; Tue, 06 Jul 2021 00:55:16 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Akshay Joshi Date: Tue, 6 Jul 2021 13:25:05 +0530 Message-ID: Subject: Re: [pgAdmin][Patch] - RM #5940 - Add support for Oauth 2 authentication To: Khushboo Vashi Cc: pgadmin-hackers Content-Type: multipart/alternative; boundary="00000000000016e38205c66fbec0" X-CLOUD-SEC-AV-Info: enterprisedb,google_mail,monitor X-CLOUD-SEC-AV-Sent: true X-Gm-Spam: 0 X-Gm-Phishy: 0 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --00000000000016e38205c66fbec0 Content-Type: text/plain; charset="UTF-8" Thanks, the patch applied. On Tue, Jul 6, 2021 at 12:01 PM Khushboo Vashi < khushboo.vashi@enterprisedb.com> wrote: > Hi Akshay, > > Please find the attached updated patch. > > Thanks, > Khushboo > On Mon, Jul 5, 2021 at 5:39 PM Akshay Joshi > 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 . >> >> 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.vashi@enterprisedb.com> wrote: >> >>> Hi, >>> >>> Please find the attached rebased patch. >>> >>> Thanks, >>> Khushboo >>> >>> On Thu, Jun 17, 2021 at 4:22 PM Khushboo Vashi < >>> khushboo.vashi@enterprisedb.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 * >> >> *Mobile: +91 976-788-8246* >> > -- *Thanks & Regards* *Akshay Joshi* *pgAdmin Hacker | Principal Software Architect* *EDB Postgres * *Mobile: +91 976-788-8246* --00000000000016e38205c66fbec0 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Thanks, the patch applied.

On Tue, Jul 6, 2021 at 12:01 PM = Khushboo Vashi <khush= boo.vashi@enterprisedb.com> wrote:
Hi Akshay,

Please find the attached updated patch.

= Thanks,
Khushboo
On Mon, Jul 5, 2021 at 5:39 PM Akshay Joshi <akshay.joshi@en= terprisedb.com> wrote:
Hi Khushboo

Following are= the review comments:

Scenario :=C2= =A0AUTHENTICATION_SOURCES =3D=C2=A0['oauth2',=C2=A0'internal']
    =
  • Don't set the OAUTH2_DISPLAY_NAME, Login button shows '= Login with None' which should be change. Maybe placeholder like <oau= th2 display name>.
Fix= ed=C2=A0
  • Enter the email address = and password. Click on the "Login with ..." button, internal serv= er error displayed on the page, we should throw a proper error message.=C2=A0
Fixed
=
Code Review:
  • Empty "__init__.py&qu= ot; file is added in the web folder, I think that is not required.
By mistake, it was added.
  • Any reas= on=C2=A0to change messages.pot file manually? It will be updated whe= n we update the message catalog.
I m= ust have run make docs, removed.=C2=A0
  • Fixed SonarQube issues in = the new file 'oauth2.py' and 'test_oauth2_with_mocking.py'.= =C2=A0Remove unused imports from both files.
Fixed=C2=A0
  • Add copyright header to "kerberos.js&qu= ot; file.
Fixed=C2=A0

Documentation:
  • Remove "of the" f= rom the description of OAUTH2_NAME it is duplicated.
  • String "A= uthorisation" should be "Authorization".
  • Instead of = providing "Oauth2 secret", "Oauth2 base url" .... can w= e provide some more detailed=C2=A0information?

Fixed.=C2=A0
Note:= I haven't tested the patch with the proper=C2=A0OAuth2 configuration.<= /div>

On Mon, Jun 28, 2021 at 4:52 PM Khushboo Vashi <khushboo.vashi@ente= rprisedb.com> wrote:
Hi,

Please find the attache= d rebased patch.

Thanks,
Khushboo
<= /div>
O= n Thu, Jun 17, 2021 at 4:22 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com= > wrote:
=
Hi,

Please find the attached patch for = the RM #5940 - Add support for Oauth 2 authentication.=C2=A0

=
The patch includes:

- Pluggable Oauth 2= authentication support=C2=A0with the multiple providers
- Config= urable support for Master Key in Oauth2 and Kerberos authentication, so use= r can save the database server password
- Introduced the separate= module/blueprint for kerberos and Oauth2 under authentication module for t= he readability and maintainability

Note: The initi= al patch for Oauth2 authentication was sent by Florian Sabonchi, I have wor= ked on top of it.


Thanks,
Khushboo



--
Thank= s & Regards
Akshay Joshi
pgAdmin Hacker | Principal Softw= are Architect
EDB Po= stgres
Mobile: +91 976-788-8246



--
Thanks & Regards
Akshay Joshi
pgAdmi= n Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246

--00000000000016e38205c66fbec0--