pgAdmin 4 commit: Fixed accessibility issue for the missing label from

2020-04-02 Thread Akshay Joshi
Fixed accessibility issue for the missing label from the table header.

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=4036f2a0f2a94047bc3e3b65b2324934baaab43f
Author: Vishal Sawale 

Modified Files
--
web/pgadmin/static/js/backform.pgadmin.js |  4 +++-
web/pgadmin/static/js/backgrid.pgadmin.js | 11 +++
2 files changed, 14 insertions(+), 1 deletion(-)



pgAdmin 4 commit: Ensure that switch cell is in sync with switch contro

2020-04-02 Thread Akshay Joshi
Ensure that switch cell is in sync with switch control for accessibility. Fixes 
#5314

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=37f55ccc25caf51451afd6ee13c5f72a0de11825
Author: Murtuza Zabuawala 

Modified Files
--
docs/en_US/release_notes_4_21.rst |  3 +-
web/pgadmin/static/js/backgrid.pgadmin.js | 48 ++-
2 files changed, 36 insertions(+), 15 deletions(-)



Re: [Accessibility] Parse & validate the web pages

2020-04-02 Thread Akshay Joshi
Thanks, patch applied.

On Wed, Apr 1, 2020 at 7:00 PM Vishal Sawale 
wrote:

> Hi Hackers,
>
> Attached is a patch for accessibility issue for table header label
> missing. If it is not present, then it will add span for screen reader.
> Please review.
>
> Regards,
> Vishal
>
>
>
>

-- 
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*


Re: [pgAdmin4][RM#5314] Make Switch cell sync with Switch control for accessibility

2020-04-02 Thread Akshay Joshi
Thanks, patch applied.

On Wed, Apr 1, 2020 at 6:58 PM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> This patch will allow the screen reader software to read the actual value
> from Switch cell like Yes/No same as we have for Switch control.
>
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>

-- 
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*


Re: [pgAdmin4][Patch] - RM 2186 - Support external authentication sources [LDAP]

2020-04-02 Thread Khushboo Vashi
Hi,

Resending the patch.
Missed the requirements.txt file in the previous patch.

Thanks,
Khushboo

On Wed, Apr 1, 2020 at 5:38 PM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Please find the attached updated patch which includes the review comments
> given in the review meeting:
>
> 1. Do not store password for ldap user in sqlite database
> 2. Forgot Password : Give error to ldap users
> 3. User Management dialog changes
> 4. Authentication source display besides username / email after login
>
> Thanks,
> Khushboo
>
>
> On Tue, Mar 24, 2020 at 3:20 PM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Please disregard my previous patch, attached the updated patch. :)
>>
>>
>> On Tue, Mar 24, 2020 at 10:32 AM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Please disregard my previous patch, attached the updated patch.
>>>
>>> On Tue, Mar 24, 2020 at 10:29 AM Khushboo Vashi <
>>> khushboo.va...@enterprisedb.com> wrote:
>>>
 Hi,

 Please find the attached updated patch.


 On Tue, Mar 17, 2020 at 4:11 PM Dave Page  wrote:

> Hi
>
> On Tue, Mar 17, 2020 at 10:24 AM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> Thanks for the review.
>>
>> On Tue, Mar 17, 2020 at 3:42 PM Dave Page  wrote:
>>
>>> Hi
>>>
>>> 30 second read of the first version of the patch...
>>>
>>> - Please move the configuration into config.py. Users should never
>>> have to modify a distributed file (it messes up packaging). I don't see 
>>> any
>>> reason to use a different file just for auth config.
>>>
>>> There are many settings for the LDAP, and in the future we will add
>> other external sources also, so I thought it would be better if we have
>> different file for the authentication.
>>
>
> Sure, but our config file is small compared to many. Splitting things
> out is more confusing for users. If they want to do that themselves of
> course, they can add a config_local.py file which includes other files as
> needed.
>
 Fixed.

>
>
>> - I think all config options should be prefixed with LDAP_ as we may
>>> have things like CERT_FILE for other purposes too.
>>>
>>> Sure.
>>
> Done.

> - I don't see any test cases.
>>>
>>> I will think about this, as right now no idea how to write test
>> cases for this.
>>
>
> It should be fairly straightforward to write tests for some of the
> functions in the auth classes. For testing the actual LDAP stuff, we
> probably need to add LDAP config options to test_config.json, and only if
> present, run the tests. That would probably need to support a list of LDAP
> servers, so we can test with different configurations (LDAP, LDAPS,
> LDAP_STARTTLS, AD etc).
>
>
 Done.

 Thanks,
 Khushboo

> Thanks.
>>>
>>> Thanks,
>> Khushboo
>>
>>>
>>> On Tue, Mar 17, 2020 at 8:55 AM Khushboo Vashi <
>>> khushboo.va...@enterprisedb.com> wrote:
>>>
 Hi,

 Please find the attached patch to support LDAP Authentication in
 Server mode.
 To test the patch, config_auth.py needs to be configured for LDAP
 configurations. The config settings are explained in this file in 
 detail.
 After configuring the parameters, start the pgadmin server in Server 
 mode
 and connect with LDAP server with the valid user via login page.

 I have tested this patch with ldap and ldap + ssl/tls. With the
 TLS, I have used the default config of ldap3 without certificates.

 @Dave, can you please review this patch, as you have a better
 understanding of LDAP and you can easily pointed out if I have missed
 anything.

 Note: For the document update I will create the task and assign to
 Nidhi for the same.

 Thanks,
 Khushboo

>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>



RM_2186_v3.patch
Description: Binary data


pgAdmin 4 v4.20 released

2020-04-02 Thread Akshay Joshi
The pgAdmin Development Team is pleased to announce pgAdmin 4 version 4.20.
This release of pgAdmin 4 includes over 13 bug fixes and new features. For
more details please see the release notes at:

https://www.pgadmin.org/docs/pgadmin4/dev/release_notes_4_20.html.

pgAdmin is the leading Open Source graphical management tool for
PostgreSQL. For more information, please see:

https://www.pgadmin.org/

Notable changes in this release include:

   - Added support of Collation, FTS Configuration, FTS Dictionary, FTS
   Parser and FTS Template to the Schema Diff.
   - Added support of Domain, Domain Constraints, and Types to the Schema
   Diff.
   - Added and fixed gettext usage for better translation coverage.
   - Improve logic to get the DDL statements as a part of the comparison.
   - Enhance the color of switch control for both light and dark theme.
   - Change some colors and opacity to comply with WCAG color contrast
   standards.
   - Fixed tab key navigation issue for Grant Wizard.
   - Fix an issue where the user can not change the value of DateTime
   picker control using the keyboard.


Builds for Windows and macOS are available now, along with a Python Wheel,
Docker Container and source code tarball from:

https://www.pgadmin.org/download/

RPM and DEB packages are expected to be available on the
postgresql.org repositories
within the next few days.

--
Akshay Joshi
pgAdmin Project


pgAdmin 4 commit: Tag REL-4_20 has been created.

2020-04-02 Thread Akshay Joshi
Tag REL-4_20 has been created.
View: 
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=tag;h=refs/tags/REL-4_20

Log Message
---
Tag v4.20

Re: [pgAdmin][RM2172] Search Objects Functionality

2020-04-02 Thread Khushboo Vashi
Hi Aditya,

Review comments:

*UI:*

1. When no object is found, the default message should be given, currently
no message displayed.
2. Can we have a tooltip on the row "Double click to locate the object in
the browser" ?
3. Full stop is missing in the message column objects are disabled in the
browser. You can enable them in the preferences dialog ( :D )
and also, we should start the statement with the capital letter.
4. If possible, use the multilevel partition table symbol same as the
browser tree.
5. gettext is missing from the search grid header.
6. Suggestion: The search button should be at the end (after type
combobox).  The current position of the controls suggest that search for
the objects and then filter it out but that's not the case.
After changing the type, we have to click on the search button.
In the current positioning, we should fetch all the records from the
backend and then filter those out depending on the type at the client side
only, so that will reduce the server requests and slickgrid is efficient it
do so.

*Backend:*

1. We do have the list of blueprint, so we can use that list instead of
taking the hard coe list in the init method of SearchObjectsHelper class.
2. While searching the object, we create an object of SearchObjectsHelper
on each request. We can create it once while initializing and utilize it on
every search.

Note: The functionality is working fine.

Thanks,
Khushboo





On Thu, Apr 2, 2020 at 9:31 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

>
>
> On Wed, Apr 1, 2020 at 6:00 PM Akshay Joshi 
> wrote:
>
>> Hi Khushboo,
>>
>> Can you please review it.
>>
> I am on it.
>
>>
>> On Mon, Mar 30, 2020 at 2:39 PM Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> Hi Hackers,
>>>
>>> Attached is the patch to implement search objects functionality in
>>> pgadmin.
>>> The feature will allow a user to search for any object in a database.
>>> Highlights of the feature:
>>> 1) Search any object with user input text with at least 3 characters.
>>> 2) Search can be done on a specific object type by selecting from the
>>> types dropdown.
>>> 3) The search results grid will show object name, object type and the
>>> object path on the browser tree. On double clicking the record, it will
>>> locate that object on the browser tree. The columns object name and type
>>> are sortable.
>>> 4) The object nodes which are disabled (hidden) using preferences will
>>> not be visible in the types dropdown. However, in the case of all types,
>>> the search records will be visible for those types greyed out.
>>> 5) You can also access search objects dialog using the button on the
>>> browser toolbar.
>>>
>>> Python and JS test cases added. Docs updated.
>>> Please review.
>>>
>>> --
>>> Thanks and Regards,
>>> Aditya Toshniwal
>>> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
>>> "Don't Complain about Heat, Plant a TREE"
>>>
>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>>
>> *Sr. Software Architect*
>> *EnterpriseDB Software India Private Limited*
>> *Mobile: +91 976-788-8246*
>>
>


Re: [pgAdmin][RM2172] Search Objects Functionality

2020-04-02 Thread Aditya Toshniwal
Hi Khushboo,

Thank you for reviewing.


On Thu, Apr 2, 2020 at 4:09 PM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi Aditya,
>
> Review comments:
>
> *UI:*
>
> 1. When no object is found, the default message should be given, currently
> no message displayed.
> 2. Can we have a tooltip on the row "Double click to locate the object in
> the browser" ?
> 3. Full stop is missing in the message column objects are disabled in the
> browser. You can enable them in the preferences dialog ( :D )
> and also, we should start the statement with the capital letter.
> 4. If possible, use the multilevel partition table symbol same as the
> browser tree.
> 5. gettext is missing from the search grid header.
>
I'll fix all above.

> 6. Suggestion: The search button should be at the end (after type
> combobox).  The current position of the controls suggest that search for
> the objects and then filter it out but that's not the case.
>
I've actually kept the most frequently used controls together. The
probability of using the types filter is less and a user would generally go
for full search. This is how even we generally do. We search first and then
apply filter if required

> After changing the type, we have to click on the search button.
> In the current positioning, we should fetch all the records from the
> backend and then filter those out depending on the type at the client side
> only, so that will reduce the server requests and slickgrid is efficient it
> do so.
>
I'll look into this. My only concern is the data may be outdated, but I
agree to filter in slickgrid on type change. The user can hit search again
if required.


> *Backend:*
>
> 1. We do have the list of blueprint, so we can use that list instead of
> taking the hard coe list in the init method of SearchObjectsHelper class.
>
The reason is, we do not support all objects for search objects. Only
objects under a database are supported. The probability of node type change
is very less.

> 2. While searching the object, we create an object of SearchObjectsHelper
> on each request. We can create it once while initializing and utilize it on
> every search.
>
The intention is to keep SearchObjectsHelper stateless. The object is
created based on the request data and it is easier to maintain
independently.

>
> Note: The functionality is working fine.
>
Great. Thanks.

>
> Thanks,
> Khushboo
>
>
>
>
>
> On Thu, Apr 2, 2020 at 9:31 AM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>>
>>
>> On Wed, Apr 1, 2020 at 6:00 PM Akshay Joshi <
>> akshay.jo...@enterprisedb.com> wrote:
>>
>>> Hi Khushboo,
>>>
>>> Can you please review it.
>>>
>> I am on it.
>>
>>>
>>> On Mon, Mar 30, 2020 at 2:39 PM Aditya Toshniwal <
>>> aditya.toshni...@enterprisedb.com> wrote:
>>>
 Hi Hackers,

 Attached is the patch to implement search objects functionality in
 pgadmin.
 The feature will allow a user to search for any object in a database.
 Highlights of the feature:
 1) Search any object with user input text with at least 3 characters.
 2) Search can be done on a specific object type by selecting from the
 types dropdown.
 3) The search results grid will show object name, object type and the
 object path on the browser tree. On double clicking the record, it will
 locate that object on the browser tree. The columns object name and type
 are sortable.
 4) The object nodes which are disabled (hidden) using preferences will
 not be visible in the types dropdown. However, in the case of all types,
 the search records will be visible for those types greyed out.
 5) You can also access search objects dialog using the button on the
 browser toolbar.

 Python and JS test cases added. Docs updated.
 Please review.

 --
 Thanks and Regards,
 Aditya Toshniwal
 pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
 "Don't Complain about Heat, Plant a TREE"

>>>
>>>
>>> --
>>> *Thanks & Regards*
>>> *Akshay Joshi*
>>>
>>> *Sr. Software Architect*
>>> *EnterpriseDB Software India Private Limited*
>>> *Mobile: +91 976-788-8246*
>>>
>>

-- 
Thanks and Regards,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"


[pgAdmin][RM5197] Accessibility issues of external packages

2020-04-02 Thread Aditya Toshniwal
Hi Hackers,

Attached patch will replace the existing color picker -
spectrum-colorpicker with @simonwep/pickr.
spectrum-colorpicker has accessibility issues and is not maintained since
long. @simonwep/pickr is actively maintained and the package is improving
its accessibility actively.

Please review.

-- 
Thanks and Regards,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"


RM5197.pickr.patch
Description: Binary data


Re: [pgAdmin][RM2172] Search Objects Functionality

2020-04-02 Thread Khushboo Vashi
On Thu, Apr 2, 2020 at 4:30 PM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:

> Hi Khushboo,
>
> Thank you for reviewing.
>
>
> On Thu, Apr 2, 2020 at 4:09 PM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi Aditya,
>>
>> Review comments:
>>
>> *UI:*
>>
>> 1. When no object is found, the default message should be given,
>> currently no message displayed.
>> 2. Can we have a tooltip on the row "Double click to locate the object in
>> the browser" ?
>> 3. Full stop is missing in the message column objects are disabled in
>> the browser. You can enable them in the preferences dialog ( :D )
>> and also, we should start the statement with the capital letter.
>> 4. If possible, use the multilevel partition table symbol same as the
>> browser tree.
>> 5. gettext is missing from the search grid header.
>>
> I'll fix all above.
>
>> 6. Suggestion: The search button should be at the end (after type
>> combobox).  The current position of the controls suggest that search for
>> the objects and then filter it out but that's not the case.
>>
> I've actually kept the most frequently used controls together. The
> probability of using the types filter is less and a user would generally go
> for full search. This is how even we generally do. We search first and then
> apply filter if required
>
Right, so type based search on slickgrid data would be useful.

> After changing the type, we have to click on the search button.
>> In the current positioning, we should fetch all the records from the
>> backend and then filter those out depending on the type at the client side
>> only, so that will reduce the server requests and slickgrid is efficient it
>> do so.
>>
> I'll look into this. My only concern is the data may be outdated, but I
> agree to filter in slickgrid on type change. The user can hit search again
> if required.
>
>
>> *Backend:*
>>
>> 1. We do have the list of blueprint, so we can use that list instead of
>> taking the hard coe list in the init method of SearchObjectsHelper class.
>>
> The reason is, we do not support all objects for search objects. Only
> objects under a database are supported. The probability of node type change
> is very less.
>
True but we can maintain the skip list (which would be less) and we do have
bluprint start with NODE, so it will be easier to fetch.

> 2. While searching the object, we create an object of SearchObjectsHelper
>> on each request. We can create it once while initializing and utilize it on
>> every search.
>>
> The intention is to keep SearchObjectsHelper stateless. The object is
> created based on the request data and it is easier to maintain
> independently.
>
>>
>> Note: The functionality is working fine.
>>
> Great. Thanks.
>
>>
>> Thanks,
>> Khushboo
>>
>>
>>
>>
>>
>> On Thu, Apr 2, 2020 at 9:31 AM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>>
>>>
>>> On Wed, Apr 1, 2020 at 6:00 PM Akshay Joshi <
>>> akshay.jo...@enterprisedb.com> wrote:
>>>
 Hi Khushboo,

 Can you please review it.

>>> I am on it.
>>>

 On Mon, Mar 30, 2020 at 2:39 PM Aditya Toshniwal <
 aditya.toshni...@enterprisedb.com> wrote:

> Hi Hackers,
>
> Attached is the patch to implement search objects functionality in
> pgadmin.
> The feature will allow a user to search for any object in a database.
> Highlights of the feature:
> 1) Search any object with user input text with at least 3 characters.
> 2) Search can be done on a specific object type by selecting from the
> types dropdown.
> 3) The search results grid will show object name, object type and the
> object path on the browser tree. On double clicking the record, it will
> locate that object on the browser tree. The columns object name and type
> are sortable.
> 4) The object nodes which are disabled (hidden) using preferences will
> not be visible in the types dropdown. However, in the case of all types,
> the search records will be visible for those types greyed out.
> 5) You can also access search objects dialog using the button on the
> browser toolbar.
>
> Python and JS test cases added. Docs updated.
> Please review.
>
> --
> Thanks and Regards,
> Aditya Toshniwal
> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
> "Don't Complain about Heat, Plant a TREE"
>


 --
 *Thanks & Regards*
 *Akshay Joshi*

 *Sr. Software Architect*
 *EnterpriseDB Software India Private Limited*
 *Mobile: +91 976-788-8246*

>>>
>
> --
> Thanks and Regards,
> Aditya Toshniwal
> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
> "Don't Complain about Heat, Plant a TREE"
>


Re: [pgAdmin4][Patch] - RM 2186 - Support external authentication sources [LDAP]

2020-04-02 Thread Akshay Joshi
Hi Khushboo

Following are the initial review comments (GUI):

*Desktop Mode: *

   - KeyError: '_auth_source_manager_obj' in desktop mode. (*Note* error
   occurs when the patch has applied and server mode is False.)

*Server Mode:*

AUTHENTICATION_SOURCES = ['internal']


   - Try to add a new user with the same email address, it throws a
   unique key constraint error. Validation was there previously before saving
   it.

AUTHENTICATION_SOURCES = ['internal', 'ldap']

   - Try to add a new user with the same email address, it throws
   unique key constraint error which should not it may possible that the user
   has the same email address for internal and ldap.

AUTHENTICATION_SOURCES = ['ldap']

   - If ipAddress or Port is not set in the configuration file then browser
   showing the following data, it should be shown proper error message on the
   login page
  - {"success":0,"errormsg":"Port could not be cast to integer value as
  ''","info":"","result":null,"data":null}
   - If IP address and port is provided but it is wrong, it shows the
   following error, can we make a generic error message? Also clicking on the
   Close button on that error message is not working.
   [image: Screenshot 2020-04-02 at 4.23.55 PM.png]
   - IP address and port of LDAP server are correct, give wrong user name
   and password, it shows error "Error binding to the LDAP Server: None".
   Please correct the appropriate error message.
   - All the configuration parameter is correct and tries to log in on LDAP
   server using username (*not email address*) and password got following
   error:

current_user.email.split('@')[0] if config.SERVER_MODE is True
AttributeError: 'NoneType' object has no attribute 'split'.

Not able to test due to the above error. Please fix and resend the patch.

On Thu, Apr 2, 2020 at 2:06 PM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Resending the patch.
> Missed the requirements.txt file in the previous patch.
>
> Thanks,
> Khushboo
>
> On Wed, Apr 1, 2020 at 5:38 PM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Please find the attached updated patch which includes the review comments
>> given in the review meeting:
>>
>> 1. Do not store password for ldap user in sqlite database
>> 2. Forgot Password : Give error to ldap users
>> 3. User Management dialog changes
>> 4. Authentication source display besides username / email after login
>>
>> Thanks,
>> Khushboo
>>
>>
>> On Tue, Mar 24, 2020 at 3:20 PM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Please disregard my previous patch, attached the updated patch. :)
>>>
>>>
>>> On Tue, Mar 24, 2020 at 10:32 AM Khushboo Vashi <
>>> khushboo.va...@enterprisedb.com> wrote:
>>>
 Please disregard my previous patch, attached the updated patch.

 On Tue, Mar 24, 2020 at 10:29 AM Khushboo Vashi <
 khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Please find the attached updated patch.
>
>
> On Tue, Mar 17, 2020 at 4:11 PM Dave Page  wrote:
>
>> Hi
>>
>> On Tue, Mar 17, 2020 at 10:24 AM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> Thanks for the review.
>>>
>>> On Tue, Mar 17, 2020 at 3:42 PM Dave Page  wrote:
>>>
 Hi

 30 second read of the first version of the patch...

 - Please move the configuration into config.py. Users should never
 have to modify a distributed file (it messes up packaging). I don't 
 see any
 reason to use a different file just for auth config.

 There are many settings for the LDAP, and in the future we will add
>>> other external sources also, so I thought it would be better if we have
>>> different file for the authentication.
>>>
>>
>> Sure, but our config file is small compared to many. Splitting things
>> out is more confusing for users. If they want to do that themselves of
>> course, they can add a config_local.py file which includes other files as
>> needed.
>>
> Fixed.
>
>>
>>
>>> - I think all config options should be prefixed with LDAP_ as we may
 have things like CERT_FILE for other purposes too.

 Sure.
>>>
>> Done.
>
>> - I don't see any test cases.

 I will think about this, as right now no idea how to write test
>>> cases for this.
>>>
>>
>> It should be fairly straightforward to write tests for some of the
>> functions in the auth classes. For testing the actual LDAP stuff, we
>> probably need to add LDAP config options to test_config.json, and only if
>> present, run the tests. That would probably need to support a list of 
>> LDAP
>> servers, so we can test with different configurations (LDAP, LDAPS,
>> LDAP_STARTTLS, AD etc).
>>
>>
> Done.
>
> Thanks,
>>>

Re: [pgAdmin][RM2172] Search Objects Functionality

2020-04-02 Thread Aditya Toshniwal
Hi Khushboo,

On Thu, Apr 2, 2020 at 4:49 PM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

>
>
> On Thu, Apr 2, 2020 at 4:30 PM Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Hi Khushboo,
>>
>> Thank you for reviewing.
>>
>>
>> On Thu, Apr 2, 2020 at 4:09 PM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi Aditya,
>>>
>>> Review comments:
>>>
>>> *UI:*
>>>
>>> 1. When no object is found, the default message should be given,
>>> currently no message displayed.
>>> 2. Can we have a tooltip on the row "Double click to locate the object
>>> in the browser" ?
>>> 3. Full stop is missing in the message column objects are disabled in
>>> the browser. You can enable them in the preferences dialog ( :D )
>>> and also, we should start the statement with the capital letter.
>>> 4. If possible, use the multilevel partition table symbol same as the
>>> browser tree.
>>> 5. gettext is missing from the search grid header.
>>>
>> I'll fix all above.
>>
>>> 6. Suggestion: The search button should be at the end (after type
>>> combobox).  The current position of the controls suggest that search for
>>> the objects and then filter it out but that's not the case.
>>>
>> I've actually kept the most frequently used controls together. The
>> probability of using the types filter is less and a user would generally go
>> for full search. This is how even we generally do. We search first and then
>> apply filter if required
>>
> Right, so type based search on slickgrid data would be useful.
>
👍

> After changing the type, we have to click on the search button.
>>> In the current positioning, we should fetch all the records from the
>>> backend and then filter those out depending on the type at the client side
>>> only, so that will reduce the server requests and slickgrid is efficient it
>>> do so.
>>>
>> I'll look into this. My only concern is the data may be outdated, but I
>> agree to filter in slickgrid on type change. The user can hit search again
>> if required.
>>
>
>>> *Backend:*
>>>
>>> 1. We do have the list of blueprint, so we can use that list instead of
>>> taking the hard coe list in the init method of SearchObjectsHelper
>>> class.
>>>
>> The reason is, we do not support all objects for search objects. Only
>> objects under a database are supported. The probability of node type change
>> is very less.
>>
> True but we can maintain the skip list (which would be less) and we do
> have bluprint start with NODE, so it will be easier to fetch.
>
I would prefer the "in" list rather than "skip" list. Each time a new node
is added to pgAdmin, we will have to update the skip list in search
objects. With the "in" list, search objects has better control.

> 2. While searching the object, we create an object of SearchObjectsHelper
>>> on each request. We can create it once while initializing and utilize it on
>>> every search.
>>>
>> The intention is to keep SearchObjectsHelper stateless. The object is
>> created based on the request data and it is easier to maintain
>> independently.
>>
>>>
>>> Note: The functionality is working fine.
>>>
>> Great. Thanks.
>>
>>>
>>> Thanks,
>>> Khushboo
>>>
>>>
>>>
>>>
>>>
>>> On Thu, Apr 2, 2020 at 9:31 AM Khushboo Vashi <
>>> khushboo.va...@enterprisedb.com> wrote:
>>>


 On Wed, Apr 1, 2020 at 6:00 PM Akshay Joshi <
 akshay.jo...@enterprisedb.com> wrote:

> Hi Khushboo,
>
> Can you please review it.
>
 I am on it.

>
> On Mon, Mar 30, 2020 at 2:39 PM Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Hi Hackers,
>>
>> Attached is the patch to implement search objects functionality in
>> pgadmin.
>> The feature will allow a user to search for any object in a database.
>> Highlights of the feature:
>> 1) Search any object with user input text with at least 3 characters.
>> 2) Search can be done on a specific object type by selecting from the
>> types dropdown.
>> 3) The search results grid will show object name, object type and the
>> object path on the browser tree. On double clicking the record, it will
>> locate that object on the browser tree. The columns object name and type
>> are sortable.
>> 4) The object nodes which are disabled (hidden) using preferences
>> will not be visible in the types dropdown. However, in the case of all
>> types, the search records will be visible for those types greyed out.
>> 5) You can also access search objects dialog using the button on the
>> browser toolbar.
>>
>> Python and JS test cases added. Docs updated.
>> Please review.
>>
>> --
>> Thanks and Regards,
>> Aditya Toshniwal
>> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
>> "Don't Complain about Heat, Plant a TREE"
>>
>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
>
> *Sr. Software Architect*
> *EnterpriseD

Re: [pgAdmin4][Patch] - RM 2186 - Support external authentication sources [LDAP]

2020-04-02 Thread Khushboo Vashi
Hi Akshay,

Please find the attached updated patch.

On Thu, Apr 2, 2020 at 4:55 PM Akshay Joshi 
wrote:

> Hi Khushboo
>
> Following are the initial review comments (GUI):
>
> *Desktop Mode: *
>
>- KeyError: '_auth_source_manager_obj' in desktop mode. (*Note* error
>occurs when the patch has applied and server mode is False.)
>
> Fixed.

> *Server Mode:*
>
> AUTHENTICATION_SOURCES = ['internal']
>
>
>- Try to add a new user with the same email address, it throws a
>unique key constraint error. Validation was there previously before saving
>it.
>
> Fixed.

> AUTHENTICATION_SOURCES = ['internal', 'ldap']
>
>- Try to add a new user with the same email address, it throws
>unique key constraint error which should not it may possible that the user
>has the same email address for internal and ldap.
>
> If the source is internal, it will not allow but with ldap, we can add the
user with the same email id.

> AUTHENTICATION_SOURCES = ['ldap']
>
>- If ipAddress or Port is not set in the configuration file then
>browser showing the following data, it should be shown proper error message
>on the login page
>   - {"success":0,"errormsg":"Port could not be cast to integer value
>   as ''","info":"","result":null,"data":null}
>
> Done

>
>- If IP address and port is provided but it is wrong, it shows the
>following error, can we make a generic error message? Also clicking on the
>Close button on that error message is not working.
>[image: Screenshot 2020-04-02 at 4.23.55 PM.png]
>
> I will look into the close button issue as it is an existing issue.

>
>-
>- IP address and port of LDAP server are correct, give wrong user name
>and password, it shows error "Error binding to the LDAP Server: None".
>Please correct the appropriate error message.
>
> Fixed.

>
>- All the configuration parameter is correct and tries to log in on
>LDAP server using username (*not email address*) and password got following
>error:
>
> current_user.email.split('@')[0] if config.SERVER_MODE is True
> AttributeError: 'NoneType' object has no attribute 'split'.
>
> Fixed.

> Not able to test due to the above error. Please fix and resend the patch.
>

Thanks,
Khushboo

Thanks,
Khushboo

>
> On Thu, Apr 2, 2020 at 2:06 PM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Resending the patch.
>> Missed the requirements.txt file in the previous patch.
>>
>> Thanks,
>> Khushboo
>>
>> On Wed, Apr 1, 2020 at 5:38 PM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> Please find the attached updated patch which includes the review
>>> comments given in the review meeting:
>>>
>>> 1. Do not store password for ldap user in sqlite database
>>> 2. Forgot Password : Give error to ldap users
>>> 3. User Management dialog changes
>>> 4. Authentication source display besides username / email after login
>>>
>>> Thanks,
>>> Khushboo
>>>
>>>
>>> On Tue, Mar 24, 2020 at 3:20 PM Khushboo Vashi <
>>> khushboo.va...@enterprisedb.com> wrote:
>>>
 Please disregard my previous patch, attached the updated patch. :)


 On Tue, Mar 24, 2020 at 10:32 AM Khushboo Vashi <
 khushboo.va...@enterprisedb.com> wrote:

> Please disregard my previous patch, attached the updated patch.
>
> On Tue, Mar 24, 2020 at 10:29 AM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Please find the attached updated patch.
>>
>>
>> On Tue, Mar 17, 2020 at 4:11 PM Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Tue, Mar 17, 2020 at 10:24 AM Khushboo Vashi <
>>> khushboo.va...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 Thanks for the review.

 On Tue, Mar 17, 2020 at 3:42 PM Dave Page 
 wrote:

> Hi
>
> 30 second read of the first version of the patch...
>
> - Please move the configuration into config.py. Users should never
> have to modify a distributed file (it messes up packaging). I don't 
> see any
> reason to use a different file just for auth config.
>
> There are many settings for the LDAP, and in the future we will
 add other external sources also, so I thought it would be better if we 
 have
 different file for the authentication.

>>>
>>> Sure, but our config file is small compared to many. Splitting
>>> things out is more confusing for users. If they want to do that 
>>> themselves
>>> of course, they can add a config_local.py file which includes other 
>>> files
>>> as needed.
>>>
>> Fixed.
>>
>>>
>>>
 - I think all config options should be prefixed with LDAP_ as we
> may have things like CERT_FILE for other purposes too.
>
> Sure.

>>> Done.
>>
>>> - I don't see any test case