Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

[Khushboo Vashi]

Hi,

Please ignore my previous patch, attached the updated one.

Thanks,
Khushboo

On Thu, Jan 14, 2021 at 12:17 PM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Please find the attached updated patch.
>
> Thanks,
> Khushboo
>
> On Thu, Jan 14, 2021 at 12:00 PM Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Hi Khushboo
>>
>> Seems you have attached the wrong patch. Please send the updated patch.
>>
>> On Wed, Jan 13, 2021 at 2:35 PM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> Please find the attached updated patch.
>>>
>>> Thanks,
>>> Khushboo
>>>
>>> On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal <
>>> aditya.toshni...@enterprisedb.com> wrote:
>>>
 Hi Khushboo,

 I've just done the code review. Apart from below, the patch looks good
 to me:

 1) Move the auth source constants -ldap, kerberos out of app object.
 They don't belong there. You can create the constants somewhere else and
 import them.

 +app.PGADMIN_LDAP_AUTH_SOURCE = 'ldap'

 +app.PGADMIN_KERBEROS_AUTH_SOURCE = 'kerberos'


 Done
>>>
 2) Are we going to make kerberos default for wsgi ?

 *--- a/web/pgAdmin4.wsgi*

 *+++ b/web/pgAdmin4.wsgi*

 @@ -24,6 +24,10 @@ builtins.SERVER_MODE = True



  import config



 +

 +config.AUTHENTICATION_SOURCES = ['kerberos']

 +config.KERBEROS_AUTO_CREATE_USER = True

 +


 Removed, it was only for testing.
>>>
 3) Remove the commented code.

 +# if self.form.data['email'] and
 self.form.data['password'] and \

 +# source.get_source_name() ==\

 +# current_app.PGADMIN_KERBEROS_AUTH_SOURCE:

 +# continue


 Removed the comment, it is actually the part of the code.
>>>
 4) KERBEROSAuthentication could be KerberosAuthentication

 class KERBEROSAuthentication(BaseAuthentication):


 Done.
>>>
 5) You can use the constants (ldap, kerberos) you had defined when
 creating a user.

 +'auth_source': 'kerberos'


 Done.
>>>
 6) The below URLs belong to the authenticate module. Currently they are
 in the browser module. I would also suggest rephrasing the URL from
 /kerberos_login to /login/kerberos. Same for logout.

>>> Done the rephrasing as well as moved to the authentication module.
>>>
>>>
 Also, even though the method GET works, we should use the POST method
 for login and DELETE for logout.

>>> Kerberos_login just redirects the page to the actual login, so no need
>>> for the POST method.
>>> I followed the same method for the Logout user we have used for the
>>> normal user.
>>>
>>>
 +@blueprint.route("/kerberos_login",

 + endpoint="kerberos_login", methods=["GET"])


 +@blueprint.route("/kerberos_logout",

 + endpoint="kerberos_logout", methods=["GET"])




>>>
 On Tue, Dec 22, 2020 at 6:07 PM Akshay Joshi <
 akshay.jo...@enterprisedb.com> wrote:

> Hi Aditya
>
> Can you please do the code review?
>
> On Tue, Dec 22, 2020 at 3:44 PM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Please find the attached patch to support Kerberos Authentication in
>> pgAdmin RM 5457.
>>
>> The patch introduces a new pluggable option for Kerberos
>> authentication, using SPNEGO to forward kerberos tickets through a 
>> browser
>> which will bypass the login page entirely if the Kerberos Authentication
>> succeeds.
>>
>> The complete setup of the Kerberos Server + pgAdmin Server + Client
>> is documented in a separate file and attached.
>>
>> This patch also includes the small fix related to logging #5829
>>
>> Thanks,
>> Khushboo
>>
>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
> *pgAdmin Hacker | Principal Software Architect*
> *EDB Postgres *
>
> *Mobile: +91 976-788-8246*
>


 --
 Thanks,
 Aditya Toshniwal
 pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
 
 "Don't Complain about Heat, Plant a TREE"

>>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>> *pgAdmin Hacker | Principal Software Architect*
>> *EDB Postgres *
>>
>> *Mobile: +91 976-788-8246*
>>
>


RM_5457_v3.patch
Description: Binary data

pgAdmin 4 commit: 1) Added support for Kerberos authentication, using S

[Akshay Joshi]

1) Added support for Kerberos authentication, using SPNEGO to forward the 
Kerberos tickets through a browser. Fixes #5457
2) Fixed incorrect log information for AUTHENTICATION_SOURCES. Fixes #5829

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=6ead597b434cb9ff9f8500c49a5c68bf9e52ab2a
Author: Khushboo Vashi 

Modified Files
--
docs/en_US/release_notes_4_30.rst  |   2 +
requirements.txt   |   1 +
web/config.py  |  22 +++-
web/pgAdmin4.py|  11 +-
web/pgadmin/__init__.py|   9 ++
web/pgadmin/authenticate/__init__.py   |  82 +++-
web/pgadmin/authenticate/internal.py   |  10 +-
web/pgadmin/authenticate/kerberos.py   | 138 +
web/pgadmin/authenticate/ldap.py   |   6 +-
web/pgadmin/browser/__init__.py|  23 +++-
.../browser/server_groups/servers/__init__.py  |  13 +-
.../servers/templates/servers/password.html|   2 +-
.../servers/templates/servers/tunnel_password.html |   4 +-
.../browser/templates/browser/kerberos_login.html  |  16 +++
.../browser/templates/browser/kerberos_logout.html |  16 +++
.../browser/tests/test_kerberos_with_mocking.py| 104 
web/pgadmin/tools/datagrid/__init__.py |   5 +-
web/pgadmin/tools/user_management/__init__.py  |  25 ++--
web/pgadmin/utils/constants.py |   9 ++
web/pgadmin/utils/master_password.py   |   8 +-
.../python_test_utils/csrf_test_client.py  |   6 +-
web/regression/runtests.py |   2 +-
22 files changed, 474 insertions(+), 40 deletions(-)

pgAdmin 4 commit: Ensure that the account expiration date for role/user

[Akshay Joshi]

Ensure that the account expiration date for role/user can’t be older than the 
current date. Fixes #6120

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=9a47e574e315ebc5d2721d69718edb72bcc91b5a
Author: Nikhil Mohite 

Modified Files
--
docs/en_US/release_notes_4_30.rst  |  1 +
.../server_groups/servers/roles/static/js/role.js  | 22 ++
2 files changed, 23 insertions(+)

Re: [pgAdmin][RM-6120]: Adding/updating user should not allow to add an older date in account expires.

[Akshay Joshi]

Thanks, patch applied.

On Thu, Jan 14, 2021 at 11:28 AM Nikhil Mohite <
nikhil.moh...@enterprisedb.com> wrote:

> Hi Team,
>
> Please find the attached patch for RM-6120
> : Adding/updating user should
> not allow to add an older date in account expires.
> Added UI validation if a user enters the account expiration date manually.
>
> --
> *Thanks & Regards,*
> *Nikhil Mohite*
> *Software Engineer.*
> *EDB Postgres* 
> *Mob.No: +91-7798364578.*
>


-- 
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres *

*Mobile: +91 976-788-8246*

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

[Akshay Joshi]

Thanks, patch applied.

On Thu, Jan 14, 2021 at 1:42 PM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Please ignore my previous patch, attached the updated one.
>
> Thanks,
> Khushboo
>
> On Thu, Jan 14, 2021 at 12:17 PM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Please find the attached updated patch.
>>
>> Thanks,
>> Khushboo
>>
>> On Thu, Jan 14, 2021 at 12:00 PM Akshay Joshi <
>> akshay.jo...@enterprisedb.com> wrote:
>>
>>> Hi Khushboo
>>>
>>> Seems you have attached the wrong patch. Please send the updated patch.
>>>
>>> On Wed, Jan 13, 2021 at 2:35 PM Khushboo Vashi <
>>> khushboo.va...@enterprisedb.com> wrote:
>>>
 Hi,

 Please find the attached updated patch.

 Thanks,
 Khushboo

 On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal <
 aditya.toshni...@enterprisedb.com> wrote:

> Hi Khushboo,
>
> I've just done the code review. Apart from below, the patch looks good
> to me:
>
> 1) Move the auth source constants -ldap, kerberos out of app object.
> They don't belong there. You can create the constants somewhere else and
> import them.
>
> +app.PGADMIN_LDAP_AUTH_SOURCE = 'ldap'
>
> +app.PGADMIN_KERBEROS_AUTH_SOURCE = 'kerberos'
>
>
> Done

> 2) Are we going to make kerberos default for wsgi ?
>
> *--- a/web/pgAdmin4.wsgi*
>
> *+++ b/web/pgAdmin4.wsgi*
>
> @@ -24,6 +24,10 @@ builtins.SERVER_MODE = True
>
>
>
>  import config
>
>
>
> +
>
> +config.AUTHENTICATION_SOURCES = ['kerberos']
>
> +config.KERBEROS_AUTO_CREATE_USER = True
>
> +
>
>
> Removed, it was only for testing.

> 3) Remove the commented code.
>
> +# if self.form.data['email'] and
> self.form.data['password'] and \
>
> +# source.get_source_name() ==\
>
> +# current_app.PGADMIN_KERBEROS_AUTH_SOURCE:
>
> +# continue
>
>
> Removed the comment, it is actually the part of the code.

> 4) KERBEROSAuthentication could be KerberosAuthentication
>
> class KERBEROSAuthentication(BaseAuthentication):
>
>
> Done.

> 5) You can use the constants (ldap, kerberos) you had defined when
> creating a user.
>
> +'auth_source': 'kerberos'
>
>
> Done.

> 6) The below URLs belong to the authenticate module. Currently they
> are in the browser module. I would also suggest rephrasing the URL from
> /kerberos_login to /login/kerberos. Same for logout.
>
 Done the rephrasing as well as moved to the authentication module.


> Also, even though the method GET works, we should use the POST method
> for login and DELETE for logout.
>
 Kerberos_login just redirects the page to the actual login, so no need
 for the POST method.
 I followed the same method for the Logout user we have used for the
 normal user.


> +@blueprint.route("/kerberos_login",
>
> + endpoint="kerberos_login", methods=["GET"])
>
>
> +@blueprint.route("/kerberos_logout",
>
> + endpoint="kerberos_logout", methods=["GET"])
>
>
>
>

> On Tue, Dec 22, 2020 at 6:07 PM Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Hi Aditya
>>
>> Can you please do the code review?
>>
>> On Tue, Dec 22, 2020 at 3:44 PM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> Please find the attached patch to support Kerberos Authentication in
>>> pgAdmin RM 5457.
>>>
>>> The patch introduces a new pluggable option for Kerberos
>>> authentication, using SPNEGO to forward kerberos tickets through a 
>>> browser
>>> which will bypass the login page entirely if the Kerberos Authentication
>>> succeeds.
>>>
>>> The complete setup of the Kerberos Server + pgAdmin Server + Client
>>> is documented in a separate file and attached.
>>>
>>> This patch also includes the small fix related to logging #5829
>>>
>>> Thanks,
>>> Khushboo
>>>
>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>> *pgAdmin Hacker | Principal Software Architect*
>> *EDB Postgres *
>>
>> *Mobile: +91 976-788-8246*
>>
>
>
> --
> Thanks,
> Aditya Toshniwal
> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
> 
> "Don't Complain about Heat, Plant a TREE"
>

>>>
>>> --
>>> *Thanks & Regards*
>>> *Akshay Joshi*
>>> *pgAdmin Hacker | Principal Software Architect*
>>> *EDB Postgres *
>>>
>>> *Mobile: +91 976-788-8246*
>>>
>>

-- 
*Thanks & Regards*
*Akshay Joshi*
*pg

Re: [pgAdmin][RM-6120]: Adding/updating user should not allow to add an older date in account expires.

[Dave Page]

On Thu, Jan 14, 2021 at 8:18 AM Akshay Joshi 
wrote:

> Thanks, patch applied.
>
> On Thu, Jan 14, 2021 at 11:28 AM Nikhil Mohite <
> nikhil.moh...@enterprisedb.com> wrote:
>
>> Hi Team,
>>
>> Please find the attached patch for RM-6120
>> : Adding/updating user
>> should not allow to add an older date in account expires.
>> Added UI validation if a user enters the account expiration date manually.
>>
>
I think this needs to be reverted (and the UI fixed to allow an older date
to be selected).

Selecting a past expiry date is a perfectly valid way to create an account
that is effectively locked, for example, to allow pre-creation of roles for
staff that are yet to join.

PostgreSQL doesn't prevent this - why should we?

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: http://www.enterprisedb.com

Re: Is there a mistake in line 22 of Make.bat?

[Dave Page]

Hi

On Thu, Jan 14, 2021 at 1:20 AM Huang, Jun 
wrote:

> Hi,
>
> The following code is line 22 of Make.bat:
> if "%Platform%" == "X86" (
>
> I tried `echo %Platform%` in VS2017 x86 native Tools Command Prompt, the
> result is "x86"(not "X86").
> If it's a mistake, the attached patch adds the changes.
>

Hmm, on the 32bit build VM I get this:

C:\Program Files (x86)\Microsoft Visual C++ Build Tools>echo %Platform%
X86

C:\Program Files (x86)\Microsoft Visual C++ Build Tools>

Which explains why everything works in the production builds. Note that
that system is VS 2015 though, but the 64 bit builds are VS 2017 iirc.

Given that 32 bit Windows builds will be dropped soon, I'm inclined to
leave this as-is for now.

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: http://www.enterprisedb.com

pgAdmin 4 commit: Revert "Ensure that the account expiration date for r

[Akshay Joshi]

Revert "Ensure that the account expiration date for role/user can’t be older 
than the current date. Fixes #6120"

This reverts commit 9a47e574e315ebc5d2721d69718edb72bcc91b5a.

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=f86f58dc562a862fd2a5dd1d55426aef04541e0e

Modified Files
--
docs/en_US/release_notes_4_30.rst  |  1 -
.../server_groups/servers/roles/static/js/role.js  | 22 --
2 files changed, 23 deletions(-)

pgAdmin 4 commit: Revert "1) Added support for Kerberos authentication,

[Akshay Joshi]

Revert "1) Added support for Kerberos authentication, using SPNEGO to forward 
the Kerberos tickets through a browser. Fixes #5457"

This reverts commit 6ead597b434cb9ff9f8500c49a5c68bf9e52ab2a.

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=f0debdd513782a3cd5900e519052130110e59f59

Modified Files
--
docs/en_US/release_notes_4_30.rst  |   2 -
requirements.txt   |   1 -
web/config.py  |  22 +---
web/pgAdmin4.py|  11 +-
web/pgadmin/__init__.py|   9 --
web/pgadmin/authenticate/__init__.py   |  82 +---
web/pgadmin/authenticate/internal.py   |  10 +-
web/pgadmin/authenticate/kerberos.py   | 138 -
web/pgadmin/authenticate/ldap.py   |   6 +-
web/pgadmin/browser/__init__.py|  23 +---
.../browser/server_groups/servers/__init__.py  |  13 +-
.../servers/templates/servers/password.html|   2 +-
.../servers/templates/servers/tunnel_password.html |   4 +-
.../browser/templates/browser/kerberos_login.html  |  16 ---
.../browser/templates/browser/kerberos_logout.html |  16 ---
.../browser/tests/test_kerberos_with_mocking.py| 104 
web/pgadmin/tools/datagrid/__init__.py |   5 +-
web/pgadmin/tools/user_management/__init__.py  |  25 ++--
web/pgadmin/utils/constants.py |   9 --
web/pgadmin/utils/master_password.py   |   8 +-
.../python_test_utils/csrf_test_client.py  |   6 +-
web/regression/runtests.py |   2 +-
22 files changed, 40 insertions(+), 474 deletions(-)

Re: [pgAdmin][RM-6120]: Adding/updating user should not allow to add an older date in account expires.

[Akshay Joshi]

Hi Nikhil

On Thu, Jan 14, 2021 at 2:38 PM Dave Page  wrote:

>
>
> On Thu, Jan 14, 2021 at 8:18 AM Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Thanks, patch applied.
>>
>> On Thu, Jan 14, 2021 at 11:28 AM Nikhil Mohite <
>> nikhil.moh...@enterprisedb.com> wrote:
>>
>>> Hi Team,
>>>
>>> Please find the attached patch for RM-6120
>>> : Adding/updating user
>>> should not allow to add an older date in account expires.
>>> Added UI validation if a user enters the account expiration date
>>> manually.
>>>
>>
> I think this needs to be reverted (and the UI fixed to allow an older date
> to be selected).
>

   Please fixed as suggested by Dave and send the patch. I'll update the RM

>
> Selecting a past expiry date is a perfectly valid way to create an account
> that is effectively locked, for example, to allow pre-creation of roles for
> staff that are yet to join.
>
> PostgreSQL doesn't prevent this - why should we?
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EDB: http://www.enterprisedb.com
>
>

-- 
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres *

*Mobile: +91 976-788-8246*

Re: [pgAdmin][RM5912]: Added support for Logical Replication.

[Akshay Joshi]

Hi Pradip

The patch is applied, but not working. Please check and send the patch
again.

On Wed, Jan 13, 2021 at 9:08 AM Pradip Parkale <
pradip.park...@enterprisedb.com> wrote:

> Hi Akshay,
>
> Please ignore my previous patch. Please find my updated patch.
>
>
> On Mon, Jan 11, 2021 at 5:07 PM Pradip Parkale <
> pradip.park...@enterprisedb.com> wrote:
>
>> Hi Hackers,
>>
>> Please find the attached patch for logical replication support.
>>
>> --
>> Thanks & Regards,
>> Pradip Parkale
>> Software Engineer | EnterpriseDB Corporation
>>
>
>
> --
> Thanks & Regards,
> Pradip Parkale
> Software Engineer | EnterpriseDB Corporation
>


-- 
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres *

*Mobile: +91 976-788-8246*

Re: [pgAdmin][RM-6120]: Adding/updating user should not allow to add an older date in account expires.

[Nikhil Mohite]

Hi Akshay,

Please find the updated patch, updated UI to allow users to select an older
date for the account expires.


Regards,
Nikhil Mohite.

On Thu, Jan 14, 2021 at 2:52 PM Akshay Joshi 
wrote:

> Hi Nikhil
>
> On Thu, Jan 14, 2021 at 2:38 PM Dave Page  wrote:
>
>>
>>
>> On Thu, Jan 14, 2021 at 8:18 AM Akshay Joshi <
>> akshay.jo...@enterprisedb.com> wrote:
>>
>>> Thanks, patch applied.
>>>
>>> On Thu, Jan 14, 2021 at 11:28 AM Nikhil Mohite <
>>> nikhil.moh...@enterprisedb.com> wrote:
>>>
 Hi Team,

 Please find the attached patch for RM-6120
 : Adding/updating user
 should not allow to add an older date in account expires.
 Added UI validation if a user enters the account expiration date
 manually.

>>>
>> I think this needs to be reverted (and the UI fixed to allow an older
>> date to be selected).
>>
>
>Please fixed as suggested by Dave and send the patch. I'll update the
> RM
>
>>
>> Selecting a past expiry date is a perfectly valid way to create an
>> account that is effectively locked, for example, to allow pre-creation of
>> roles for staff that are yet to join.
>>
>> PostgreSQL doesn't prevent this - why should we?
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EDB: http://www.enterprisedb.com
>>
>>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
> *pgAdmin Hacker | Principal Software Architect*
> *EDB Postgres *
>
> *Mobile: +91 976-788-8246*
>


RM_6120_v2.patch
Description: Binary data

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

[Dave Page]

Hi Khushboo,

As you know, this has been rolled back as the buildfarm blew up. I think
there are a number of TODOs that need to be addressed, given that the
gssapi Python module is dependent on MIT Kerberos:

In the patch:

- Linux packages will need the additional dependencies to be declared in
the RPM/DEBs.
- The setup scripts for Linux will need to have the -dev packages added as
appropriate.
- The various READMEs that describe how to build packages will need to be
updated.
- The Dockerfile will need to be modified to add the required packages.
- The Windows build will need to be updated so the installer ships
additional required DLLs.
- Are there any additional macOS dependencies? If so, they need to be
handled.

In the buildfarm:

- All Linux build VMs need to be updated with the additional dependencies.
- On Windows, we need to figure out how to build/ship KfW. It's a pain to
build, which we would typically do ourselves to ensure we're consistently
using the same buildchain. If we do build it ourselves:
  - Will the Python package find it during it's build?
  - We'll need to create a Jenkins job to perform the build.
- Is any work required on macOS, or does it ship with everything that's
needed? If not, we'll need to build it, and create the Jenkins job.

One final thought: on Windows/macOS, can we force a binary installation
from PIP (pip install --only-binary=gssapi gssapi)? If so, will that
include the required libraries, as psycopg2-binary does?


On Thu, Jan 14, 2021 at 8:18 AM Akshay Joshi 
wrote:

> Thanks, patch applied.
>
> On Thu, Jan 14, 2021 at 1:42 PM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Please ignore my previous patch, attached the updated one.
>>
>> Thanks,
>> Khushboo
>>
>> On Thu, Jan 14, 2021 at 12:17 PM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> Please find the attached updated patch.
>>>
>>> Thanks,
>>> Khushboo
>>>
>>> On Thu, Jan 14, 2021 at 12:00 PM Akshay Joshi <
>>> akshay.jo...@enterprisedb.com> wrote:
>>>
 Hi Khushboo

 Seems you have attached the wrong patch. Please send the updated patch.

 On Wed, Jan 13, 2021 at 2:35 PM Khushboo Vashi <
 khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Please find the attached updated patch.
>
> Thanks,
> Khushboo
>
> On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Hi Khushboo,
>>
>> I've just done the code review. Apart from below, the patch looks
>> good to me:
>>
>> 1) Move the auth source constants -ldap, kerberos out of app object.
>> They don't belong there. You can create the constants somewhere else and
>> import them.
>>
>> +app.PGADMIN_LDAP_AUTH_SOURCE = 'ldap'
>>
>> +app.PGADMIN_KERBEROS_AUTH_SOURCE = 'kerberos'
>>
>>
>> Done
>
>> 2) Are we going to make kerberos default for wsgi ?
>>
>> *--- a/web/pgAdmin4.wsgi*
>>
>> *+++ b/web/pgAdmin4.wsgi*
>>
>> @@ -24,6 +24,10 @@ builtins.SERVER_MODE = True
>>
>>
>>
>>  import config
>>
>>
>>
>> +
>>
>> +config.AUTHENTICATION_SOURCES = ['kerberos']
>>
>> +config.KERBEROS_AUTO_CREATE_USER = True
>>
>> +
>>
>>
>> Removed, it was only for testing.
>
>> 3) Remove the commented code.
>>
>> +# if self.form.data['email'] and
>> self.form.data['password'] and \
>>
>> +# source.get_source_name() ==\
>>
>> +# current_app.PGADMIN_KERBEROS_AUTH_SOURCE:
>>
>> +# continue
>>
>>
>> Removed the comment, it is actually the part of the code.
>
>> 4) KERBEROSAuthentication could be KerberosAuthentication
>>
>> class KERBEROSAuthentication(BaseAuthentication):
>>
>>
>> Done.
>
>> 5) You can use the constants (ldap, kerberos) you had defined when
>> creating a user.
>>
>> +'auth_source': 'kerberos'
>>
>>
>> Done.
>
>> 6) The below URLs belong to the authenticate module. Currently they
>> are in the browser module. I would also suggest rephrasing the URL from
>> /kerberos_login to /login/kerberos. Same for logout.
>>
> Done the rephrasing as well as moved to the authentication module.
>
>
>> Also, even though the method GET works, we should use the POST method
>> for login and DELETE for logout.
>>
> Kerberos_login just redirects the page to the actual login, so no need
> for the POST method.
> I followed the same method for the Logout user we have used for the
> normal user.
>
>
>> +@blueprint.route("/kerberos_login",
>>
>> + endpoint="kerberos_login", methods=["GET"])
>>
>>
>> +@blueprint.route("/kerberos_logout",
>>
>> + endpoin

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

[Dave Page]

FYI, I did a quick test (and browse of PyPI):

- On Windows, it seems there is a binary wheel available:

(gssapi) C:\Users\dpage>pip install gssapi
Collecting gssapi
  Downloading gssapi-1.6.12-cp39-cp39-win_amd64.whl (670 kB)
 || 670 kB 3.3 MB/s
Collecting decorator
  Downloading decorator-4.4.2-py2.py3-none-any.whl (9.2 kB)
Installing collected packages: decorator, gssapi
Successfully installed decorator-4.4.2 gssapi-1.6.12

- On macOS, the wheel is built by pip, but it doesn't seem to have any
additional binary dependencies.

This should simplify things a lot - we just need to ensure the build
scripts use the binary package on Windows, and install the build deps on
the Linux/Docker environments (and update the package builds with the
additional dependencies of course).


On Thu, Jan 14, 2021 at 4:04 PM Dave Page  wrote:

> Hi Khushboo,
>
> As you know, this has been rolled back as the buildfarm blew up. I think
> there are a number of TODOs that need to be addressed, given that the
> gssapi Python module is dependent on MIT Kerberos:
>
> In the patch:
>
> - Linux packages will need the additional dependencies to be declared in
> the RPM/DEBs.
> - The setup scripts for Linux will need to have the -dev packages added as
> appropriate.
> - The various READMEs that describe how to build packages will need to be
> updated.
> - The Dockerfile will need to be modified to add the required packages.
> - The Windows build will need to be updated so the installer ships
> additional required DLLs.
> - Are there any additional macOS dependencies? If so, they need to be
> handled.
>
> In the buildfarm:
>
> - All Linux build VMs need to be updated with the additional dependencies.
> - On Windows, we need to figure out how to build/ship KfW. It's a pain to
> build, which we would typically do ourselves to ensure we're consistently
> using the same buildchain. If we do build it ourselves:
>   - Will the Python package find it during it's build?
>   - We'll need to create a Jenkins job to perform the build.
> - Is any work required on macOS, or does it ship with everything that's
> needed? If not, we'll need to build it, and create the Jenkins job.
>
> One final thought: on Windows/macOS, can we force a binary installation
> from PIP (pip install --only-binary=gssapi gssapi)? If so, will that
> include the required libraries, as psycopg2-binary does?
>
>
> On Thu, Jan 14, 2021 at 8:18 AM Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Thanks, patch applied.
>>
>> On Thu, Jan 14, 2021 at 1:42 PM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> Please ignore my previous patch, attached the updated one.
>>>
>>> Thanks,
>>> Khushboo
>>>
>>> On Thu, Jan 14, 2021 at 12:17 PM Khushboo Vashi <
>>> khushboo.va...@enterprisedb.com> wrote:
>>>
 Hi,

 Please find the attached updated patch.

 Thanks,
 Khushboo

 On Thu, Jan 14, 2021 at 12:00 PM Akshay Joshi <
 akshay.jo...@enterprisedb.com> wrote:

> Hi Khushboo
>
> Seems you have attached the wrong patch. Please send the updated patch.
>
> On Wed, Jan 13, 2021 at 2:35 PM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Please find the attached updated patch.
>>
>> Thanks,
>> Khushboo
>>
>> On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> Hi Khushboo,
>>>
>>> I've just done the code review. Apart from below, the patch looks
>>> good to me:
>>>
>>> 1) Move the auth source constants -ldap, kerberos out of app object.
>>> They don't belong there. You can create the constants somewhere else and
>>> import them.
>>>
>>> +app.PGADMIN_LDAP_AUTH_SOURCE = 'ldap'
>>>
>>> +app.PGADMIN_KERBEROS_AUTH_SOURCE = 'kerberos'
>>>
>>>
>>> Done
>>
>>> 2) Are we going to make kerberos default for wsgi ?
>>>
>>> *--- a/web/pgAdmin4.wsgi*
>>>
>>> *+++ b/web/pgAdmin4.wsgi*
>>>
>>> @@ -24,6 +24,10 @@ builtins.SERVER_MODE = True
>>>
>>>
>>>
>>>  import config
>>>
>>>
>>>
>>> +
>>>
>>> +config.AUTHENTICATION_SOURCES = ['kerberos']
>>>
>>> +config.KERBEROS_AUTO_CREATE_USER = True
>>>
>>> +
>>>
>>>
>>> Removed, it was only for testing.
>>
>>> 3) Remove the commented code.
>>>
>>> +# if self.form.data['email'] and
>>> self.form.data['password'] and \
>>>
>>> +# source.get_source_name() ==\
>>>
>>> +# current_app.PGADMIN_KERBEROS_AUTH_SOURCE:
>>>
>>> +# continue
>>>
>>>
>>> Removed the comment, it is actually the part of the code.
>>
>>> 4) KERBEROSAuthentication could be KerberosAuthentication
>>>
>>> class KERBEROSAuthentication(BaseAuthentication):
>>>

Re: Quick search for menu items & help articles

[Dave Page]

Hi

On Thu, Jan 14, 2021 at 4:48 PM Pramod Ahire 
wrote:

> Hi Team,
>
>
>
> Please find the attached designs & patch that contains complete
> functionality except below to do for quick search.
>
>
>
> To Do:
>
>
>
>- Unit test cases are not that sufficient to cover complete code, but
>will be working in background to cover up those one
>- In pgadmin, for disabled menu items we need to add info that will
>describe why menu has disabled & how it will be enabled. Either another way
>to enable all of them & show respective reason in popup that menu is
>disabled for.
>
>
>
> Please do let me know if I missed anything or suggestion of yours.
>

Looks very good. I haven't done an extensive code review/test, but two
things spring to mind immediately:

1) I think the search box should be the top item on the Help menu. I do not
think it should be on the far end of the menu bar, as it looks too much
like it will search for data (think of search on a website).

2) Do we need another loading icon? Surely there's one in the source tree
already that we can use?

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: http://www.enterprisedb.com

RE: Is there a mistake in line 22 of Make.bat?

[Huang, Jun]

Thanks for your answer.

--
Thanks & Regards
HuangJ

From: Dave Page 
Sent: Thursday, January 14, 2021 5:15 PM
To: Huang, Jun/黄 军 
Cc: pgadmin-hack...@postgresql.org
Subject: Re: Is there a mistake in line 22 of Make.bat?

Hi

On Thu, Jan 14, 2021 at 1:20 AM Huang, Jun 
mailto:huangj.f...@cn.fujitsu.com>> wrote:
Hi,

The following code is line 22 of Make.bat:
if "%Platform%" == "X86" (

I tried `echo %Platform%` in VS2017 x86 native Tools Command Prompt, the result 
is "x86"(not "X86").
If it's a mistake, the attached patch adds the changes.

Hmm, on the 32bit build VM I get this:

C:\Program Files (x86)\Microsoft Visual C++ Build Tools>echo %Platform%
X86

C:\Program Files (x86)\Microsoft Visual C++ Build Tools>

Which explains why everything works in the production builds. Note that that 
system is VS 2015 though, but the 64 bit builds are VS 2017 iirc.

Given that 32 bit Windows builds will be dropped soon, I'm inclined to leave 
this as-is for now.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: http://www.enterprisedb.com

pgAdmin 4 commit: Ensure that the user should be able to specify an old

[Akshay Joshi]

Ensure that the user should be able to specify an older date for the account 
expiration of the role/user. Fixes #6120

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=a2edf317a9ce02c16058feb229c3228a03cf145a
Author: Nikhil Mohite 

Modified Files
--
docs/en_US/release_notes_4_30.rst  |  1 +
.../server_groups/servers/roles/static/js/role.js  |  1 +
web/pgadmin/static/js/backform.pgadmin.js  | 18 ++
3 files changed, 16 insertions(+), 4 deletions(-)

Re: [pgAdmin][RM-6120]: Adding/updating user should not allow to add an older date in account expires.

[Akshay Joshi]

Thanks, patch applied.

On Thu, Jan 14, 2021 at 5:58 PM Nikhil Mohite <
nikhil.moh...@enterprisedb.com> wrote:

> Hi Akshay,
>
> Please find the updated patch, updated UI to allow users to select an
> older date for the account expires.
>
>
> Regards,
> Nikhil Mohite.
>
> On Thu, Jan 14, 2021 at 2:52 PM Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Hi Nikhil
>>
>> On Thu, Jan 14, 2021 at 2:38 PM Dave Page  wrote:
>>
>>>
>>>
>>> On Thu, Jan 14, 2021 at 8:18 AM Akshay Joshi <
>>> akshay.jo...@enterprisedb.com> wrote:
>>>
 Thanks, patch applied.

 On Thu, Jan 14, 2021 at 11:28 AM Nikhil Mohite <
 nikhil.moh...@enterprisedb.com> wrote:

> Hi Team,
>
> Please find the attached patch for RM-6120
> : Adding/updating user
> should not allow to add an older date in account expires.
> Added UI validation if a user enters the account expiration date
> manually.
>

>>> I think this needs to be reverted (and the UI fixed to allow an older
>>> date to be selected).
>>>
>>
>>Please fixed as suggested by Dave and send the patch. I'll update the
>> RM
>>
>>>
>>> Selecting a past expiry date is a perfectly valid way to create an
>>> account that is effectively locked, for example, to allow pre-creation of
>>> roles for staff that are yet to join.
>>>
>>> PostgreSQL doesn't prevent this - why should we?
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EDB: http://www.enterprisedb.com
>>>
>>>
>>
>> --
>> *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*

[pgAdmin][RM-6122]: There is no informative message when there is no diff found

[Nikhil Mohite]

Hi Team,

Please find the attached patch for RM-6122
: There is no informative
message when there is no diff found.

-- 
*Thanks & Regards,*
*Nikhil Mohite*
*Software Engineer.*
*EDB Postgres* 
*Mob.No: +91-7798364578.*


RM_6122.patch
Description: Binary data

Re: Quick search for menu items & help articles

[Akshay Joshi]

Hi Pramod

Following are the review comments:

   - linter issue "Undefined variable $black"
   - In Desktop mode remove the separator beside the search icon.


On Thu, Jan 14, 2021 at 10:18 PM Pramod Ahire 
wrote:

> Hi Team,
>
>
>
> Please find the attached designs & patch that contains complete
> functionality except below to do for quick search.
>
>
>
> To Do:
>
>
>
>- Unit test cases are not that sufficient to cover complete code, but
>will be working in background to cover up those one
>- In pgadmin, for disabled menu items we need to add info that will
>describe why menu has disabled & how it will be enabled. Either another way
>to enable all of them & show respective reason in popup that menu is
>disabled for.
>
>
>
> Please do let me know if I missed anything or suggestion of yours.
>
>
>
> Thanks !
>
>
>
> *Pramod Ahire*
>
> *Software Engineer*
>
>
>
> [image:
> https://lh4.googleusercontent.com/U1erEuyI_d0xEUA0CrKjwx9aWQ52HNCxc38dMsLP-ZrLgfVNrhsrNobxlmeOdb1kMPtrrxcUwEHZgbGJC4R0qR4r1sBZa_z9R8ihFRaP2Hr_Wnhq6HcIQHe1ZoviDPwUkTdzNcg]
>
>
>
> C: +91-020-66449600/601
>
> D: +91-9028697679
>
> *edbpostgres.com *
>
>
>


-- 
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres *

*Mobile: +91 976-788-8246*

Re: Quick search for menu items & help articles

[Pramod Ahire]

Hi Dave,

 

Thank you for comments ! I had described points below. Please do let me know if 
anything missing.

 

From: Dave Page 
Date: Thursday, 14 January 2021 at 10:26 PM
To: Pramod Ahire 
Cc: pgadmin-hackers 
Subject: Re: Quick search for menu items & help articles

 

Hi

 

On Thu, Jan 14, 2021 at 4:48 PM Pramod Ahire  
wrote:

Hi Team, 

 

Please find the attached designs & patch that contains complete functionality 
except below to do for quick search.

 

To Do: 

 
Unit test cases are not that sufficient to cover complete code, but will be 
working in background to cover up those one
In pgadmin, for disabled menu items we need to add info that will describe why 
menu has disabled & how it will be enabled. Either another way to enable all of 
them & show respective reason in popup that menu is disabled for. 
 

Please do let me know if I missed anything or suggestion of yours.

 

Looks very good. I haven't done an extensive code review/test, but two things 
spring to mind immediately:

 

1) I think the search box should be the top item on the Help menu. I do not 
think it should be on the far end of the menu bar, as it looks too much like it 
will search for data (think of search on a website).

 

- As we are showing menu items as well in search results, it can be redundant 
for end user & increase duplications of menu items. Please advise your thoughts 
on this.

 

2) Do we need another loading icon? Surely there's one in the source tree 
already that we can use? 

 

- As we are loading help articles count in background, so I have added 
background loading icon to show near to count of results. Our existing icon is 
of blue & white circle combination, which will be more useful to show 
foreground loading.

 

Please do let me know your valuable inputs on this. 

 

 

Pramod Ahire

Software Engineer

 

 

C: +91-020-66449600/601

D: +91-9028697679

edbpostgres.com