Issue 7783

Title
Allow to use session key instead of login/password
Priority
feature
Status
resolved
Nosy list
ced, nblock, reviewbot, roundup-bot, semarie
Assigned to
ced
Keywords
review

Created on 2018-10-14.15:32:02 by ced, last changed 2 months ago by roundup-bot.

Files

File name Uploaded Type Details
xmlrpc-transport-headers.patch ced, 2018-10-16.10:52:05 text/plain view
session-subclassing.diff semarie, 2018-10-16.10:27:47 text/plain view

Messages

New changeset 0175b779611d by Cédric Krier in branch 'default':
Support session with XML-RPC
https://hg.tryton.org/tryton-env/rev/0175b779611d
New changeset 6bc3cc2d62e1 by Cédric Krier in branch 'default':
Support session with XML-RPC
https://hg.tryton.org/proteus/rev/6bc3cc2d62e1
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-09-01.18:53:20

Here is review350521003 which relies on the headers parameter.

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2019-02-19.17:25:03
The PR has been accepted, it will be part of Python 3.8: https://github.com/python/cpython/commit/beda52ed36e701e45f22903fc4d3bec0d085b25b
So we can think about having a patch for 5.4 which relies on this new feature but taking care that it will only work for >=python3.8
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2018-11-03.12:36:16
Here is the PR for Python: https://bugs.python.org/issue35153
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2018-10-16.10:52:05
On 2018-10-16 10:27, Sebastien Marie wrote:
> > This sounds quiet complex and requires a lot of specific code.
> > At the end, it is only about putting the right Authorization headers. I find that the Transport from xmlrpc.client could be a little bit more flexible and allow to take extra headers as parameter. I think it will be good to try to improve Python stdlib first.
> 
> xmlrpc.client has two classes: Transport and SafeTransport, and we
> should to deal with it. I agree that it isn't really flexible.
> 
> but it isn't really a code duplication, even if we should provide
> SessionTransport and SessionSafeTransport classes. SessionSafeTransport
> could inherit from SessionTransport + SafeTransport to have the logic
> for session handling while keeping the one for SSL.

By duplication, I mean the code in ServerProxy.__init__ that decides
which Transport to use will need to be duplicated. That's why I prefer
that we improve Python stdlib instead. See the attached patch.

> > For the login/logout, they should already be available as common.db.login and common.db.logout.
> 
> yes they are available, but usage for common.db.login is a bit complex
> to figure without example. Maybe proteus README should contains an
> example of proper usage.

Indeed I think some glue code to nicely manage it is welcomed.

> I attached a minimal diff, mostly for archival purpose:
> - make XmlrpcConfig() to be usable with `with' syntax (it revokes
>   session token on exit by calling common.db.logout)
> - SessionTransport and SessionSafeTransport transport classes
> - obtain_session() function as shortcut around common.db.login
> 
> Most of them (SessionTransport, SessionSafeTransport and
> obtain_session()) could live outside proteus.
> 
> Usage could be something like:
> 
> import proteus
> from proteus import Model
> from proteus.config import SessionTransport
> 
> session = proteus.config.obtain_session('http://hostname/dbname/', 'username', 'password')
> with proteus.config.set_xmlrpc('http://hostname/dbname/', transport=SessionTransport(session)) as config:
>       Party = Model.get('party.party')
>       ...

I think it will be better that the "obtain_session" was the context
manager instead of set_xmlrpc. It will login and also set the config and
restore the previous one at exit.
Author: [hidden] (semarie)
Date: 2018-10-16.10:27:48
> This sounds quiet complex and requires a lot of specific code.
> At the end, it is only about putting the right Authorization headers. I find that the Transport from xmlrpc.client could be a little bit more flexible and allow to take extra headers as parameter. I think it will be good to try to improve Python stdlib first.

xmlrpc.client has two classes: Transport and SafeTransport, and we
should to deal with it. I agree that it isn't really flexible.

but it isn't really a code duplication, even if we should provide
SessionTransport and SessionSafeTransport classes. SessionSafeTransport
could inherit from SessionTransport + SafeTransport to have the logic
for session handling while keeping the one for SSL.

> For the login/logout, they should already be available as common.db.login and common.db.logout.

yes they are available, but usage for common.db.login is a bit complex
to figure without example. Maybe proteus README should contains an
example of proper usage.

I attached a minimal diff, mostly for archival purpose:
- make XmlrpcConfig() to be usable with `with' syntax (it revokes
  session token on exit by calling common.db.logout)
- SessionTransport and SessionSafeTransport transport classes
- obtain_session() function as shortcut around common.db.login

Most of them (SessionTransport, SessionSafeTransport and
obtain_session()) could live outside proteus.

Usage could be something like:

import proteus
from proteus import Model
from proteus.config import SessionTransport

session = proteus.config.obtain_session('http://hostname/dbname/', 'username', 'password')
with proteus.config.set_xmlrpc('http://hostname/dbname/', transport=SessionTransport(session)) as config:
      Party = Model.get('party.party')
      ...

or with ssl and specific context:

import proteus
import ssl
from proteus import Model
from proteus.config import SessionSafeTransport

context = ssl._create_default_https_context()
context.check_hostname = False
context.verify_mode = ssl.CERT_NONE

session = proteus.config.obtain_session('https://hostname/dbname/', 'username', 'password', context=context)
with proteus.config.set_xmlrpc('https://hostname/dbname/', transport=SessionSafeTransport(session, context=context)) as config:
	Party = Model.get('party.party')
	...
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2018-10-15.14:59:08
This sounds quiet complex and requires a lot of specific code.
At the end, it is only about putting the right Authorization headers. I find that the Transport from xmlrpc.client could be a little bit more flexible and allow to take extra headers as parameter. I think it will be good to try to improve Python stdlib first.

For the login/logout, they should already be available as common.db.login and common.db.logout.
Author: [hidden] (semarie)
Date: 2018-10-15.11:26:05
how the API should be designed ?

the more generic approch would be:
- a function to get a fresh session token (using URI + login + password)
- a function to get a xmlrpc.client.Transport configured to use a session token (using a session-token)
- a function to get a xmlrpc.client.SafeTransport configured to use a session token (using a session-token)
- next, set_xmlrpc() would have to be called with transport=mySessionTransport (no change)

eventually, the functions for Transport/SafeTransport could be mixed if we pass also the URI (to choose between the two). Or alternatively, using fallback method and fingerprint is also possible (to do the same as the tryton client), but I am unsure as it seems to me it is overkilled for proteus.

having distinct functions for "get a fresh token" and "get a Transport" would permit token reuse between script calls for example.

having "get a Transport" and "set_xmlrpc()" distinct functions would permit passing specific SSL context to configure the Transport.

it could be interesting to have two way to get a fresh session token:
- a function to get a fresh session token
- use `with` python-syntax and automatically get a fresh token on begining, and terminate the session on block exit (it will disallow session reuse)
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2018-10-14.15:32:01
The login check is now quite expensive as it uses slow hash methods. As this check is done on each call, it slows down script that makes a lot of calls.
The idea is to allow to set a session from a login call to the xmlrpc configuration.

Come from the discussion: http://www.tryton.org/~irclog/2018-10-14.log.html#t14:10-2
History
Date User Action Args
2021-09-23 23:16:58roundup-botsetmessages: + msg70372
2021-09-23 23:16:52roundup-botsetmessages: + msg70371
nosy: + roundup-bot
status: testing -> resolved
2021-09-03 00:06:43reviewbotsetmessages: + msg69839
2021-09-01 19:13:36reviewbotsetmessages: + msg69769
nosy: + reviewbot
2021-09-01 18:53:20cedsetassignedto: ced
keyword: + review
messages: + msg69767
reviews: 350521003
status: chatting -> testing
2019-03-16 15:26:54cedsetkeyword: - patch
2019-02-19 17:25:03cedsetmessages: + msg47119
2018-11-07 10:14:37nblocksetnosy: + nblock
2018-11-03 12:36:16cedsetmessages: + msg44751
2018-10-16 10:52:06cedsetfiles: + xmlrpc-transport-headers.patch
messages: + msg44433
2018-10-16 10:27:48semariesetfiles: + session-subclassing.diff
messages: + msg44432
keyword: + patch
2018-10-15 14:59:08cedsetmessages: + msg44400
2018-10-15 11:26:05semariesetstatus: unread -> chatting
messages: + msg44393
2018-10-14 15:43:29semariesetnosy: + semarie
2018-10-14 15:32:02cedcreate