Re: Somewhat excessive version checks

2021-01-12 Thread Dave Page
On Mon, Jan 11, 2021 at 10:06 PM Magnus Hagander 
wrote:

> Hi!
>
> If I read the code correctly, pgadmin will (unless turned off) hit the
> website to check the version.json file for updates *every time it
> starts*.
>

Every time the server starts, which is a little different, but still...


>
> Wouldn't it make sense to rate limit that to checking say once per 24
> hours maximum? Or even 48?
>

That certainly wouldn't be a bad idea.


>
> It seems nobody needs the update *that* quickly, and AFAICT it does
> call out to make that check synchronously on startup which means the
> user is waiting.
>
> And if/when doing that, it would be useful to include an
> If-Modified-Since header on the request, so the server can just
> respond with a tiny 304 reply when there is no update, which is going
> to be the majority of the time. Or possibly even more efficiently,
> create a custom etag and use If-None-Matches. If you make that etag be
> say the version that the client has, it becomes very cheap to check
> and you don't need to track any extra data.
>

Patches welcome!

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

EDB: http://www.enterprisedb.com


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

2021-01-12 Thread Dave Page
Hi

On Mon, Jan 11, 2021 at 5:42 PM Stephen Frost  wrote:

>
> Accessing systems outside of the Kerberized environment is obviously a
> different situation as you *can't* use the Kerberos credentials- and,
> hopefully, everyone is using password managers and has a distinct and
> different password for every service they do use outside of the
> Kerberized environment.  When you're talking about a set of systems
> which live inside of the Kerberized environment, however, it's simply
> not sensible to ask the user to provide their *domain-level* credentials
> which an attacker could use to log in as that user to the entire domain
> and have complete access over their account and that's exactly what is
> likely to end up being the case here because the only way to set this up
> would be Kerberos for pgAdmin and LDAP for PG- at least until delegated
> credentials are implemented.
>

Which is no worse than the current situation - in fact it's arguably better
because there's one less system that isn't Kerberised.

Don't forget, you (as the system administrator) also have the choice of
whether or not to use Kerberos. If you're not happy to have the pgAdmin
authentication be kerberised whilst the database server access is not, then
don't enable Kerberos until phase 2 is complete.


>
> > You basically seem to be saying that once a user logs into something
> using
> > Kerberos, *everything* else they login to from there must also be done
> > using Kerberos - which clearly will not be the case in the vast majority
> of
> > deployments.
>
> Everything else they login to from there in the same Kerberized
> environment absolutely should be done using Kerberos delegated
> credentials.  That's the point of Kerberos delegation.  Are you modeling
> this approach based on some existing system which accepts Kerberos
> logins but then *doesn't* allow use of delegated credentials to log into
> other Kerberized systems from there?  Surely SSH works great with
> delegated credentials, as does any website that uses mod_auth_kerb or
> mod_auth_gss, or IIS..
>
> I sure hope that the vast majority of deployments where pgAdmin is set
> up with Kerberos will be using Kerberos for logging into PG with
> delegated credentials, and further, that we will be *strongly*
> encouraging that as otherwise you might as well use LDAP auth for all of
> it and accept that any compromise of the web server or of PG will result
> in complete compromise of any user's account who accesses the system.
>

I suspect that may not be the case, or at least most people will be working
in mixed environments, e.g. Kerberos on their local network with
non-Kerberised RDS servers for example. This is certainly something I've
seen in the field many times.


>
> I don't understand all this push-back.
>

There are benefits for some users with phase one alone, so I don't see (and
still don't) a need to hold it back. It also potentially allows us to get
feedback on things that don't work as expected earlier, to minimise any
re-work that might be required. Don't forget that pgAdmin releases monthly
(except around EOY for obvious reasons), and incrementally releases and
adds features, unlike PostgreSQL.


>
> The intent is to do the 'phase 2', right?  And it hopefully will happen
> in relatively short order, no?  At least, I'd think it would make sense,
> while people have developer environments set up and working with
> Kerberos to go ahead and get that part done.  All I'm saying is that the
> 'phase 1' part really shouldn't be independently released, or if it is,
> it should be *heavily* caveated that it is strongly discouraged for
> people to run it in an environment where pgadmin and PG are in the same
> Kerberized environment because it's not possible to set that up, with
> just phase 1 done, in a manner which would avoid the pgadmin and PG
> servers seeing the user's password.
>

Phase 2 is scheduled to be done immediately.

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

EDB: http://www.enterprisedb.com


Re: [pgAdmin][RM5488] Tooltip information does not display properly if user check all options under explain analyze

2021-01-12 Thread Akshay Joshi
Hi Aditya

Code looks good to me. Below are the review comments:

   - Copy and Paste not working in the popup.
   - Documentation needs to be updated.


On Mon, Jan 11, 2021 at 4:38 PM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:

> Hi Hackers,
>
> Attached patch improves the way explain plan details tooltip for a node is
> shown. With the change, popup with details will be shown upon clicking a
> node, and it will remain open until explicitly closed.
>
> Please review.
>
> --
> 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*


pgAdmin 4 commit: Fixed an issue where target connection is wrong while

2021-01-12 Thread Akshay Joshi
Fixed an issue where target connection is wrong while checking version 
compatibility in schema diff.

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=3f089f31a3bb1e787d926c78e8f291e59454fb13
Author: Huang, Jun 

Modified Files
--
web/pgadmin/tools/schema_diff/__init__.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)



Re: The target connection was wrong in function check_version_compatibility()

2021-01-12 Thread Akshay Joshi
Thanks, patch applied.

On Tue, Jan 12, 2021 at 11:03 AM Huang, Jun 
wrote:

> Hi,
> The target connection was made by source connection manager(it should be
> made by target connection manager)
> in function
> check_version_compatibility(web\pgadmin\tools\schema_diff\__init__.py).
> Attached patch adds the changes.
>
> Regrads,
> Huang
>
>
>

-- 
*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

2021-01-12 Thread Magnus Hagander
On Tue, Jan 12, 2021 at 10:08 AM Dave Page  wrote:
>
> Hi
>
> On Mon, Jan 11, 2021 at 5:42 PM Stephen Frost  wrote:
>>
>>
>> Accessing systems outside of the Kerberized environment is obviously a
>> different situation as you *can't* use the Kerberos credentials- and,
>> hopefully, everyone is using password managers and has a distinct and
>> different password for every service they do use outside of the
>> Kerberized environment.  When you're talking about a set of systems
>> which live inside of the Kerberized environment, however, it's simply
>> not sensible to ask the user to provide their *domain-level* credentials
>> which an attacker could use to log in as that user to the entire domain
>> and have complete access over their account and that's exactly what is
>> likely to end up being the case here because the only way to set this up
>> would be Kerberos for pgAdmin and LDAP for PG- at least until delegated
>> credentials are implemented.
>
>
> Which is no worse than the current situation - in fact it's arguably better 
> because there's one less system that isn't Kerberised.
>
> Don't forget, you (as the system administrator) also have the choice of 
> whether or not to use Kerberos. If you're not happy to have the pgAdmin 
> authentication be kerberised whilst the database server access is not, then 
> don't enable Kerberos until phase 2 is complete.
>
>>
>>
>> > You basically seem to be saying that once a user logs into something using
>> > Kerberos, *everything* else they login to from there must also be done
>> > using Kerberos - which clearly will not be the case in the vast majority of
>> > deployments.
>>
>> Everything else they login to from there in the same Kerberized
>> environment absolutely should be done using Kerberos delegated
>> credentials.  That's the point of Kerberos delegation.  Are you modeling
>> this approach based on some existing system which accepts Kerberos
>> logins but then *doesn't* allow use of delegated credentials to log into
>> other Kerberized systems from there?  Surely SSH works great with
>> delegated credentials, as does any website that uses mod_auth_kerb or
>> mod_auth_gss, or IIS..
>>
>> I sure hope that the vast majority of deployments where pgAdmin is set
>> up with Kerberos will be using Kerberos for logging into PG with
>> delegated credentials, and further, that we will be *strongly*
>> encouraging that as otherwise you might as well use LDAP auth for all of
>> it and accept that any compromise of the web server or of PG will result
>> in complete compromise of any user's account who accesses the system.
>
>
> I suspect that may not be the case, or at least most people will be working 
> in mixed environments, e.g. Kerberos on their local network with 
> non-Kerberised RDS servers for example. This is certainly something I've seen 
> in the field many times.

+1. I can see a lot of cases where people would like to benefit from
the *convenience* of Kerberos login into their pgadmin, and then
continue to use a db connection that does not use Kerberos. There's
many orgs that for example have a policy that says they *must* use
passwords in to the db regardless of Kerbeos. We can argue whether
that's a smart policy or not, but it's very real, and those people
would still be able to benefit from a Kerberos login into pgadmin.

Getting those people to do kerberos into pgadmin and then password
intot he database would be a strong improvement over ldap to pgadmin
and password into the database. Sure, if the ldap password and the db
password is the same the difference isn't that big, but more often
than not the db password is independent.

RDS is a good example of this, but there are definitely plenty of
non-cloud environments who would also benefit fro that.


>> I don't understand all this push-back.
>
>
> There are benefits for some users with phase one alone, so I don't see (and 
> still don't) a need to hold it back. It also potentially allows us to get 
> feedback on things that don't work as expected earlier, to minimise any 
> re-work that might be required. Don't forget that pgAdmin releases monthly 
> (except around EOY for obvious reasons), and incrementally releases and adds 
> features, unlike PostgreSQL.
>
>>
>>
>> The intent is to do the 'phase 2', right?  And it hopefully will happen
>> in relatively short order, no?  At least, I'd think it would make sense,
>> while people have developer environments set up and working with
>> Kerberos to go ahead and get that part done.  All I'm saying is that the
>> 'phase 1' part really shouldn't be independently released, or if it is,
>> it should be *heavily* caveated that it is strongly discouraged for
>> people to run it in an environment where pgadmin and PG are in the same
>> Kerberized environment because it's not possible to set that up, with
>> just phase 1 done, in a manner which would avoid the pgadmin and PG
>> servers seeing the user's password.
>
>
> Phase 2 is scheduled to be done immediatel

pgAdmin 4 commit: Improve the explain plan details by showing popup ins

2021-01-12 Thread Akshay Joshi
Improve the explain plan details by showing popup instead of tooltip on 
clicking of the specified node. Fixes #5488

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=6589f82e777e2adef1433c9859121c98a029f1eb
Author: Aditya Toshniwal 

Modified Files
--
docs/en_US/images/query_output_explain_details.png | Bin 179156 -> 213690 bytes
docs/en_US/images/query_toolbar_explain.png| Bin 173583 -> 195038 bytes
docs/en_US/query_tool.rst  |  10 +-
docs/en_US/release_notes_4_30.rst  |   1 +
web/pgadmin/misc/static/explain/js/explain.js  | 110 -
.../misc/static/explain/js/explain_statistics.js   |  46 -
web/pgadmin/misc/static/explain/scss/_explain.scss |  25 -
web/pgadmin/static/scss/_bootstrap.overrides.scss  |   1 +
.../misc/explain/explain_statistics_spec.js|  60 ---
9 files changed, 106 insertions(+), 147 deletions(-)



Re: [pgAdmin][RM5488] Tooltip information does not display properly if user check all options under explain analyze

2021-01-12 Thread Akshay Joshi
Thanks, patch applied.

On Tue, Jan 12, 2021 at 4:08 PM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:

> Hi Akshay,
>
> Attached is the updated patch. Please review.
>
> On Tue, Jan 12, 2021 at 2:51 PM Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Hi Aditya
>>
>> Code looks good to me. Below are the review comments:
>>
>>- Copy and Paste not working in the popup.
>>- Documentation needs to be updated.
>>
>>
>> On Mon, Jan 11, 2021 at 4:38 PM Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> Hi Hackers,
>>>
>>> Attached patch improves the way explain plan details tooltip for a node
>>> is shown. With the change, popup with details will be shown upon clicking a
>>> node, and it will remain open until explicitly closed.
>>>
>>> Please review.
>>>
>>> --
>>> 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,
> 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*


[pgAdmin][RM-6075]: Non-admin user is unable to view shared server created using 'Service'.

2021-01-12 Thread Nikhil Mohite
Hi Team,

Please find the attached patch for RM-6075:
 Non-admin user is unable to
view shared server created using 'Service'.

1. Added migration to remove "Not null" constraints from a port column of
sharedserver table.
2. Updated model class of sharedserver table for the same.


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


RM_6075.patch
Description: Binary data


Re: [pgAdmin] RM4892 pgAdmin "Inception"

2021-01-12 Thread Rahul Shirsat
Hi Hackers,

Please find the patch which reverts the previous changes. Code changes do
not solve the issue, so a workaround is mentioned in the RM to cater the
issue.

On Thu, Dec 24, 2020 at 1:15 PM Akshay Joshi 
wrote:

> Thanks, patch applied.
>
> On Wed, Dec 23, 2020 at 8:23 PM Rahul Shirsat <
> rahul.shir...@enterprisedb.com> wrote:
>
>> Just forgot to mention, patch consists of one more fix of a schema not
>> loading on several refresh of browser or startup of application - *FIXED*
>>
>> [image: image.png]
>>
>> On Wed, Dec 23, 2020 at 8:16 PM Rahul Shirsat <
>> rahul.shir...@enterprisedb.com> wrote:
>>
>>> Hi Hackers,
>>>
>>> Please find the attached patch which resolves the issue of pgadmin
>>> Inception in Firefox browser.
>>>
>>> The fix is tested on:
>>>
>>>1. MacOS 10.13.6 Firefox Browser 84.0 (64-bit)
>>>2. Windows 2016 Firefox Browser 84.0.1 (64-bit)
>>>
>>> --
>>> *Rahul Shirsat*
>>> Senior Software Engineer | EnterpriseDB Corporation.
>>>
>>
>>
>> --
>> *Rahul Shirsat*
>> Senior Software Engineer | EnterpriseDB Corporation.
>>
>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
> *pgAdmin Hacker | Principal Software Architect*
> *EDB Postgres *
>
> *Mobile: +91 976-788-8246*
>


-- 
*Rahul Shirsat*
Senior Software Engineer | EnterpriseDB Corporation.


RM4892_v2.patch
Description: Binary data


Re: Somewhat excessive version checks

2021-01-12 Thread Magnus Hagander
On Tue, Jan 12, 2021 at 9:57 AM Dave Page  wrote:
>
> On Mon, Jan 11, 2021 at 10:06 PM Magnus Hagander  wrote:
>>
>> Hi!
>>
>> If I read the code correctly, pgadmin will (unless turned off) hit the
>> website to check the version.json file for updates *every time it
>> starts*.
>
>
> Every time the server starts, which is a little different, but still...

Hmm. So one of us is definitely reading things wrong then :) I see it
in the index() method, which has an URL router for / -- isn't that
called for every time somebody somebody starts their browser to it?
I'm not saying for every reload, but with a server install with 10
users, won't it do it once for each?

Or when is that actually called?


>> Wouldn't it make sense to rate limit that to checking say once per 24
>> hours maximum? Or even 48?
>
>
> That certainly wouldn't be a bad idea.
>
>>
>>
>> It seems nobody needs the update *that* quickly, and AFAICT it does
>> call out to make that check synchronously on startup which means the
>> user is waiting.
>>
>> And if/when doing that, it would be useful to include an
>> If-Modified-Since header on the request, so the server can just
>> respond with a tiny 304 reply when there is no update, which is going
>> to be the majority of the time. Or possibly even more efficiently,
>> create a custom etag and use If-None-Matches. If you make that etag be
>> say the version that the client has, it becomes very cheap to check
>> and you don't need to track any extra data.
>
>
> Patches welcome!

Hah, I clearly can't even figure out when the method is called :)

And presumably you'd also want some place to store the state between
calls, so you can keep showing the warnings about upgrades? Do you
have state storage for such things arleady=

-- 
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/




Re: Somewhat excessive version checks

2021-01-12 Thread Magnus Hagander
On Tue, Jan 12, 2021 at 5:59 AM Khushboo Vashi
 wrote:
>
>
>
> On Tue, Jan 12, 2021 at 3:36 AM Magnus Hagander  wrote:
>>
>> Hi!
>>
>> If I read the code correctly, pgadmin will (unless turned off) hit the
>> website to check the version.json file for updates *every time it
>> starts*.
>>
>> Wouldn't it make sense to rate limit that to checking say once per 24
>> hours maximum? Or even 48?
>>
>> It seems nobody needs the update *that* quickly, and AFAICT it does
>> call out to make that check synchronously on startup which means the
>> user is waiting.
>>
> Agreed, we should have some mechanism in place to limit the server hit, maybe 
> an asynchronous call from the client while loading.


Seems async front he server would be a better choice there, if you
have some ways for that? Otherwise, how to determine which user to
trust when storing the result etc?

-- 
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/




pgAdmin 4 commit: Reverting fix for #4892, updated the RM with the work

2021-01-12 Thread Akshay Joshi
Reverting fix for #4892, updated the RM with the workaround.

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=18e4b30634ad681765d5bc39c5866d342084c074
Author: Rahul Shirsat 

Modified Files
--
docs/en_US/release_notes_4_30.rst|  1 -
web/pgadmin/tools/datagrid/static/js/datagrid.js | 15 ++-
2 files changed, 2 insertions(+), 14 deletions(-)



pgAdmin 4 commit: Fixed an issue where Non-admin user is unable to view

2021-01-12 Thread Akshay Joshi
Fixed an issue where Non-admin user is unable to view shared server created 
using service. Fixes #6075

Branch
--
master

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

Modified Files
--
docs/en_US/release_notes_4_30.rst|  1 +
web/migrations/versions/a39bd015b644_.py | 97 
web/pgadmin/model/__init__.py|  3 +-
3 files changed, 99 insertions(+), 2 deletions(-)



Re: [pgAdmin] RM4892 pgAdmin "Inception"

2021-01-12 Thread Akshay Joshi
Thanks, patch applied.

On Tue, Jan 12, 2021 at 7:01 PM Rahul Shirsat <
rahul.shir...@enterprisedb.com> wrote:

> Hi Hackers,
>
> Please find the patch which reverts the previous changes. Code changes do
> not solve the issue, so a workaround is mentioned in the RM to cater the
> issue.
>
> On Thu, Dec 24, 2020 at 1:15 PM Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Thanks, patch applied.
>>
>> On Wed, Dec 23, 2020 at 8:23 PM Rahul Shirsat <
>> rahul.shir...@enterprisedb.com> wrote:
>>
>>> Just forgot to mention, patch consists of one more fix of a schema not
>>> loading on several refresh of browser or startup of application -
>>> *FIXED*
>>>
>>> [image: image.png]
>>>
>>> On Wed, Dec 23, 2020 at 8:16 PM Rahul Shirsat <
>>> rahul.shir...@enterprisedb.com> wrote:
>>>
 Hi Hackers,

 Please find the attached patch which resolves the issue of pgadmin
 Inception in Firefox browser.

 The fix is tested on:

1. MacOS 10.13.6 Firefox Browser 84.0 (64-bit)
2. Windows 2016 Firefox Browser 84.0.1 (64-bit)

 --
 *Rahul Shirsat*
 Senior Software Engineer | EnterpriseDB Corporation.

>>>
>>>
>>> --
>>> *Rahul Shirsat*
>>> Senior Software Engineer | EnterpriseDB Corporation.
>>>
>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>> *pgAdmin Hacker | Principal Software Architect*
>> *EDB Postgres *
>>
>> *Mobile: +91 976-788-8246*
>>
>
>
> --
> *Rahul Shirsat*
> Senior Software Engineer | EnterpriseDB Corporation.
>


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

*Mobile: +91 976-788-8246*


Re: [pgAdmin][RM-6075]: Non-admin user is unable to view shared server created using 'Service'.

2021-01-12 Thread Akshay Joshi
Thanks, patch applied.

On Tue, Jan 12, 2021 at 5:26 PM Nikhil Mohite <
nikhil.moh...@enterprisedb.com> wrote:

> Hi Team,
>
> Please find the attached patch for RM-6075:
>  Non-admin user is unable to
> view shared server created using 'Service'.
>
> 1. Added migration to remove "Not null" constraints from a port column of
> sharedserver table.
> 2. Updated model class of sharedserver table for the same.
>
>
> --
> *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*