Re: RM2815: Relogin to pgAdmin from sqleditor/datadrid if session exprires

Hi,

Please find rebased patch.

--
*Harshal Dhumal*
*Sr. Software Engineer*

EnterpriseDB India: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Jan 9, 2018 at 9:21 PM, Harshal Dhumal <
harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:

> Hi Dave,
>
> Please find updated patch where we are notifying user about connection
> reset.
>
> --
> *Harshal Dhumal*
> *Sr. Software Engineer*
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Mon, Jan 8, 2018 at 7:39 PM, Harshal Dhumal <
> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>
>> On Mon, Jan 8, 2018 at 7:26 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>>
>>>
>>> On Mon, Jan 8, 2018 at 1:18 PM, Harshal Dhumal <
>>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>
>>>>
>>>> On Mon, Jan 8, 2018 at 6:11 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Mon, Jan 8, 2018 at 12:37 PM, Harshal Dhumal <
>>>>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> On Mon, Jan 8, 2018 at 5:15 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>>
>>>>>>> HI
>>>>>>>
>>>>>>> On Mon, Jan 8, 2018 at 11:41 AM, Harshal Dhumal <
>>>>>>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> On Mon, Jan 8, 2018 at 4:34 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi
>>>>>>>>>
>>>>>>>>> On Fri, Jan 5, 2018 at 7:50 AM, Harshal Dhumal <
>>>>>>>>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Further details:
>>>>>>>>>>
>>>>>>>>>> 1. If session is expired and user performs any action from
>>>>>>>>>> sqleditor that makes ajax call
>>>>>>>>>> then in ajax error call back user can check and handle login
>>>>>>>>>> related error using code snippet.
>>>>>>>>>>
>>>>>>>>>> if (pgAdmin.Browser.UserManagement.is_pga_login_required(xhr)) {
>>>>>>>>>> return pgAdmin.Browser.UserManagement.pga_login();
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> Where is xhr is standard xhr or jqxhr object.
>>>>>>>>>>
>>>>>>>>>> 2. Similarly for connection lost (only maintenance db connection
>>>>>>>>>> as we can recover or reconnect other
>>>>>>>>>> connections if maintenance db connection is alive). It will
>>>>>>>>>> attempt to create/reconnect connection without
>>>>>>>>>> asking password (to handle passwordless connection, or saveed
>>>>>>>>>> password or password from pgpass file)
>>>>>>>>>> If connection to database still fails then it'll prompt for
>>>>>>>>>> password.
>>>>>>>>>>
>>>>>>>>>> Code snippet:
>>>>>>>>>> SqlEditorController.handle_connection_lost();
>>>>>>>>>> once connection lost is detected one can call
>>>>>>>>>> handle_connection_lost() to reconnect.
>>>>>>>>>>
>>>>>>>>>> 3. We maintain some additional data in session to maintain
>>>>>>>>>> affinity between
>>>>>>>>>> each sqleditor/datagrid instance to backend database connection.
>>>>>>>>>> However if session expires and user
>>>>>>>>>> re-loggins then we need to first restore affinity between
>>>>>>>>>> sqleditor to backend database before we can start
>>>>>>>>>> using query tool.
>>>>>>>>>>
>>>>>>>>>> Code snippet:
>>>>>>>>>>
>>>>>>>>>> if(is_new_transaction_required(xhr)) {
>>>>>>>>>> SqlEditorController.init_transaction();
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> (note: I haven't looked at the code yet)
>>>>>>>>>
>>>>>>>>> How does this handle re-establishment of the connection
>>>>>>>>> mid-transaction, or, if the user has modified any session variables?
>>>>>>>>>
>>>>>>>>> ServeManager and Connection Manager are written in a such way that
>>>>>>>> if any connection is lost except maintenance db connection
>>>>>>>> then we can re-connect or create new connection without prompting
>>>>>>>> for database password and if maintenance db connection is lost
>>>>>>>> then It prompts for password.
>>>>>>>>
>>>>>>>
>>>>>>> Right.
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> Regarding session variables as long as flask session is not expired
>>>>>>>> we uses same session variables. But in case of user logout (due to
>>>>>>>> flask session expire) we create new transaction id and sets new
>>>>>>>> session variables for that particular Sql editor /datagrid instance.
>>>>>>>>
>>>>>>>
>>>>>>> I mean DB session variables (and related things). For example, if
>>>>>>> the user executed queries such as the following, then they absolutely need
>>>>>>> to know if the session got reset:
>>>>>>>
>>>>>>
>>>>>> Ok.
>>>>>> Then in this case can we notify user about the same. That we're
>>>>>> unable to restore old database connection and created new one and therefore
>>>>>> any DB session variables were set/modified (like SET
>>>>>> CLIENT_ENCODING..., SET DATESTYLE...) are lost (or similar message).
>>>>>>
>>>>>>
>>>>>>
>>>>>>> CREATE TEMPORARY TABLE ....
>>>>>>> SET ROLE ...
>>>>>>> SET [various other options]
>>>>>>>
>>>>>>> If the user has done any of those things (or similar things that I
>>>>>>> haven't thought of), then we cannot just blindly reset the database
>>>>>>> connection.
>>>>>>>
>>>>>>
>>>>>> We only create new connection if flask session was expired as we lost
>>>>>> transaction id associated with Sql editor/datagrid and therefore unique
>>>>>> connection id
>>>>>> given to connection which was associated to Sql editor/datagrid. In
>>>>>> this case we can notify about same (as stated above).
>>>>>>
>>>>>
>>>>> If it's only the flask session we're resetting, not the database
>>>>> connection, we won't need to warn the user will we?
>>>>>
>>>>
>>>> The problem here is that if flask session is reset then we lose the
>>>> information about which connection was associate with which query
>>>> tool/datagrid.
>>>>
>>>> lets say If user have opened 3 query tools with same database therefore
>>>> we'll have 3 separate connections
>>>> (each will have unique connection id). Now this information that which
>>>> connection is associate with which query
>>>> tool is lost and also unique connection id. So there is no way that we
>>>> can get that connection
>>>> from connection manager ( *manager.connection(did=<some did>,
>>>> conn_id=<unique connection id>)* ).
>>>>
>>>> So even database connection was not lost and only flask session was
>>>> reset we need to create new connection. So I think we'll need to warn user.
>>>>
>>>
>>> Agreed... and ensure the database connection is fully reset.
>>>
>>>
>>>>
>>>> Also If we save connection id to client side (in browser in JS) still
>>>> we won't be able to get same connection
>>>> even though we know connection id in case of flask session reset. As
>>>> for each logged in user (pgAdmin user)
>>>> but for same database server we create separate ServerManager (and
>>>> therefore separate connection pool)
>>>> and flask session reset is same as if same user is logged in from
>>>> another browser.
>>>>
>>>>
>>>>>
>>>>> But... what if the database connection has also been lost in the
>>>>> meantime. Would we handle that?
>>>>>
>>>> We create new connection irrespective of old connection state if flask
>>>> session was reset as explained above.
>>>>
>>>
>>> OK.
>>>
>>> So... we need to always warn the user that the connection has been
>>> reset, so they know if they've lost previous GUC changes or temp tables
>>> etc, and conversely, we need to reset the database connection to ensure
>>> thatGUC changes or temp tables don't end up getting re-associated with the
>>> wrong session. Sound right?
>>>
>>> Yes
>>
>>
>>> I assume updates to the patch are required?
>>>
>> Yes. I'll send updated one.
>>
>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>