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

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