Re: Somewhat excessive version checks

[Dave Page]

On Tue, Jan 12, 2021 at 7:50 PM Magnus Hagander  wrote:

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

Huh, no you're right. It's a long time since I wrote that code :-/


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

Yes - the SQLite config database.  See pgadmin.model.

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

EDB: http://www.enterprisedb.com

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

[Khushboo Vashi]

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


RM_5457_v1.patch
Description: Binary data

pgAdmin 4 commit: Fixed syntax error in print function

[Akshay Joshi]

Fixed syntax error in print function

Branch
--
master

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

Modified Files
--
web/pgadmin/misc/bgprocess/process_executor.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

pgAdmin 4 commit: Use a working timestamp server.

[Dave Page]

Use a working timestamp server.

Branch
--
master

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

Modified Files
--
Make.bat | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

pgAdmin 4 commit: Timestamp servers are few and far between now :-/

[Dave Page]

Timestamp servers are few and far between now :-/

Branch
--
master

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

Modified Files
--
Make.bat | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

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

[Huang, Jun]

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.


Regrads,
Huang




Fix-Platfrom.patch
Description: Fix-Platfrom.patch

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

[Nikhil Mohite]

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


RM_6120.patch
Description: Binary data

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

[Akshay Joshi]

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*

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

[Khushboo Vashi]

Hi,

Please find the attached updated patch.

Thanks,
Khushboo

On Thu, Jan 14, 2021 at 12:00 PM Akshay Joshi 
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_v2.patch
Description: Binary data