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

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
>