Xpra: Ticket #1488: new auth module: sqlsqlite

Create possibility to keep the users records (like multifile records structure) in mySQL table



Tue, 04 Apr 2017 08:29:35 GMT - Antoine Martin: status, component changed; milestone set

Will be useful for things like #1486 and may allow us to move away from multifile. With a sqlite backend, we could easily provide a simple GUI too.


Wed, 19 Apr 2017 05:00:19 GMT - Antoine Martin: attachment set

stub for sql auth


Wed, 19 Apr 2017 05:08:09 GMT - Antoine Martin:

It would make sense to eventually move the common SQL code to a utility superclass, so this auth module should be called "sqliteauth".

Then we can also have a "mysqlauth" and "postgresqlauth", etc.. All the connectors should follow the PEP 249 : Python Database API Specification v2.0

The "sqliteauth" module can take a single specific parameter: the path to the database file, whereas the other SQL auth modules will require extra parameters: host, port, username, password, database name, etc All the SQL auth modules should use the "password_query" and "sessions_query" (this one can be made optional) as per the stub patch, moved to the superclass.


Mon, 24 Apr 2017 06:48:16 GMT - Denis01:

Hello, 1)In order to make the proper call and init of module i tried to get familiar with the content of "program_context"

from xpra.platform import program_context

But didn't find the code source. Could you please help to find it. 2) it seems that Sqlite doesn't support sql schemes. So 2 ways are possible:

Which one to choose? Thank you.


Mon, 24 Apr 2017 06:51:29 GMT - Antoine Martin: owner, status changed

No idea what you mean here. All SQL databases support creating a schema for sure: https://sqlite.org/lang_createtable.html.


Mon, 24 Apr 2017 08:49:29 GMT - Denis01:

1) program_context: just to avoid asking "stupid" questions. Tried to pass the DB name thru command string but failed. What is a right way to do? For example. Is that a right command?

xpra proxy --bind-tcp=XX.XX.XX.XX:443 --auth=sql:create=/root/xpra.sdb

2) SQLite. As don't know what you would like to do after with that .... my question was just simply inspired by the following discussion: http://stackoverflow.com/questions/30897377/python-sqlite3-create-a-schema-without-having-to-use-a-second-database but ok, will start with db.connect("temp DB.file"). And then you will check if it is OK or not


Mon, 24 Apr 2017 10:11:13 GMT - Antoine Martin:

You seem to be confused about what "creating the schema" means, see https://en.wikipedia.org/wiki/Database_schema. In this instance, it means creating the table(s) required for using the auth module.


Tue, 25 Apr 2017 05:48:40 GMT - Denis01:

Ok. Just I started thinking that talking about mysqlauth, postgreessql, superclass, create schema ... you meant to have a kind of abstraction for sql module. :-) Now i think that the idea of main() with "create" inside is just to create a new empty DB with the predefined structure for new installation of xpra. If it is a right thinking i'll do "create" as a first step and then "update" (for the future releases sql changes) as the second :-)


Wed, 26 Apr 2017 01:45:17 GMT - Denis01: attachment set


Wed, 26 Apr 2017 01:58:36 GMT - Denis01:

Hello, could you please check the script (basically SQL replacement for Multifile for now). If OK - I will add the function for dynamic sessions initialization and balancing. To create DB:

python /../.../sql_auth.py create [DB path]

DB path (for example) - /root/xpra.sdb

In SQL(table users) the following fields should be fulfilled: 1) user_ipn - login name 2) pass - password 3) displays (the same format as in Multifile - tcp:XX.XX.XX.XX:port) 4) env_options (optional, the same format as in Multifile) 5) session_options (optional, the same format as in Multifile)

To start xpra

xpra --bind-tcp=XX.XX.XX.XX:port --auth=sql:filename=/.../xpra.sdb

or xpra proxy...


Wed, 26 Apr 2017 04:43:15 GMT - Antoine Martin:

You've just taken the stub sql auth script I had posted, and added load balancing stuff to get_sessions - which does not belong there. You have not addressed any of the issues I had listed.


Wed, 26 Apr 2017 09:15:57 GMT - Denis01:

Hello! I apologize! was very tired and posted another file! Hope that new one is better


Wed, 26 Apr 2017 09:19:57 GMT - Denis01: attachment set


Wed, 26 Apr 2017 09:20:58 GMT - Denis01:

sql_auth_NEW.py​ is a correct one


Wed, 26 Apr 2017 10:48:44 GMT - Antoine Martin:

That's better.

Please address the following points:

Now for the "create" command code:


Wed, 26 Apr 2017 11:05:52 GMT - Denis01:

OK. will try to modify ASAP.

Field "user_conn_str" in DB stands for "user_connection_string" and should point to dynamic session initialization and has a format like "--start-child=/bin/gedit --exit-with-children" to start process (will be used in balancing module). So if the field "displays" is not empty - Static session if displays is empty and the field "user_conn_str" is not empty - Dynamic session. If both are empty - error


Wed, 26 Apr 2017 15:23:39 GMT - Denis01:

New update. Everything should be taken in account except: 1) "the init function is not needed, sqlauth does not use the legacy password file option" have not figured out how to do it without

init(opts)

as then

password_file = opts.password_file

says that object not found 2) "function (call it "create"), call this from main" and "was already a stub for it (see line 136) why place it somewhere else?" put "create" inside

with program_context("SQL Auth", "SQL Auth"):

as before for test reasons it was out of that branch.

Or it is needed to create a specific dedicated

def create()
....

function?


Wed, 26 Apr 2017 15:24:08 GMT - Denis01: attachment set


Wed, 26 Apr 2017 16:30:39 GMT - Antoine Martin:

Other things left to be done:


Wed, 26 Apr 2017 17:23:15 GMT - Denis01:

Done Comments: 1) "user_conn_str" is used (or will be used: lines 86-90) and logically it is user's data like "Display" and other settings so might be in the same table. And as a result of lines 86-90 the sessions info is delivered (the same for static session info). But if should be deleted where it has to be moved? 2) I test each time when code is modified For wiki/man - ok, sure


Wed, 26 Apr 2017 17:23:36 GMT - Denis01: attachment set


Wed, 26 Apr 2017 17:25:44 GMT - Antoine Martin:

1) it isn't used yet and as I said multiple times - this is probably not the right place for it anyway 2) we have unit tests for all auth modules:

browser/xpra/trunk/src/unittests/unit/server/auth_test.py, this one will be no exception


Wed, 26 Apr 2017 18:21:47 GMT - Denis01:

2) ok, will try to figure out how it works 1) being ready to continuer to test and etc. please give a hint where is a good place for it :-)


Thu, 27 Apr 2017 06:54:30 GMT - Antoine Martin:

Still many things to fix:

As for the right place for creating sessions, this belongs in the proxy code where it already has the ability to create new sessions - it just needs to be enhanced to be able to start remote sessions. (and I have already pointed out the function that provides this feature too)


Thu, 27 Apr 2017 07:32:12 GMT - Denis01:

Ok, sure. And other point (if you don't mind) will add a call for "create()" inside "...main...." (and keep the same call from "context" branch) as basically it doesn't work from command line for now

Concerning Sessions creation: 1) so to make the session creation function independent from the type of auth module (file, sqlite, mysql, etc) you want to include that in

start_proxy():
      try:
sessions = client_proto.authenticator.get_sessions()

Thu, 27 Apr 2017 07:42:10 GMT - Denis01:

ops (press the enter)...

right?

2) "already pointed out the function that provides this feature too" if it is

xpra start ssh/username:password@HOST:SSHPORT/DISPLAY --start=xterm --attach=no"

function. As far as I understand there is no PORT identification on which client application should start at target server. Only SSHPORT to connect.


Thu, 27 Apr 2017 07:45:28 GMT - Antoine Martin:

And other point (if you don't mind) will add a call for "create()" inside "main" (and keep the same call from "context" branch) as basically it doesn't work from command line

No. The context code doesn't change sys.argv, you must be doing something wrong.

1) so to make the session creation function independent from the type of auth module (file, sqlite, mysql, etc) you want to include that in

No. Like I said, where it already includes code to start a new session.

if it is

No it isn't.

As far as I understand there is no PORT identification on which client application should start at target server. Only SSHPORT to connect.

This is not a function, this is a command line. And anyway, it does support specifying the port: add bind-tcp= to this command.

In any case, this doesn't really belong in this sqlauth ticket.


Thu, 27 Apr 2017 08:19:37 GMT - Denis01:

Create() just to be sure that i understand correctly. In the

sql_auth_new_up2.py​

the create function moved to

with program_context("SQL Auth", "SQL Auth"):
      if cmd=="create":
              create(db_file_name)

and basically is not called from this block from the command line. So moving create(db_file_name) out of this block will allow to call it directly from command line. If you don't mind - will do that


Thu, 27 Apr 2017 08:20:40 GMT - Antoine Martin:

If you don't mind - will do that

No. Don't do that. Figure out why it isn't called from where it should be called, and fix that.


Thu, 27 Apr 2017 08:22:49 GMT - Antoine Martin:

Here's a hint: address ALL my earlier comments and it will probably be called. Indentation matters.


Thu, 27 Apr 2017 11:09:34 GMT - Denis01:

:-) seems to be resolved

Another hint is wanted :-) Concerning

init(opts)

what is "opts"? tried to find it in "sys_auth_base" but there is the same definition there


Thu, 27 Apr 2017 11:28:32 GMT - Denis01:

Just without it can't remove "init" function. Filename is not defined


Thu, 27 Apr 2017 11:38:48 GMT - Antoine Martin:

Just remove the init function completely and either replace password_file by a sensible default value (ie: "auth.sdb") or throw an exception if the "filename" attribute is not specified. This function is only used for password file authentication and legacy command line options - it will be removed soon.


Thu, 27 Apr 2017 14:00:14 GMT - Denis01:

ok. got it So that will be removed and module won't be fully (with the possibility to point DB file) operational for now. Waiting for future evolutions


Thu, 27 Apr 2017 15:52:39 GMT - Antoine Martin:

So that will be removed and module won't be fully (with the possibility to point DB file) operational for now. Waiting for future evolutions

Just leave the function in for now, but empty. You specify the database file through the filename attribute, it should be fully operational as it is.


Sat, 29 Apr 2017 05:22:23 GMT - Denis01:

Ok, done. And all the comments from "comment:19" are included.


Sat, 29 Apr 2017 05:22:42 GMT - Denis01: attachment set


Sat, 29 Apr 2017 05:38:38 GMT - Antoine Martin:

Looks good, only found two remaining problems:

Not sure why we have "Error: sqlauth. No session type is identified" in the second branch. If we don't have displays, what's the problem? The proxy may still create or find one for us. (this will be refined with the ability to start sessions).


Sat, 29 Apr 2017 06:46:11 GMT - Denis01:

:-) and data[3] is wrong. should be data[2]. just have finished testing and it didn't work. data[2]=displays

"Session type is not defined". And "sessions definition..." Now having looked at proxy_server.py and r15693 it seems that "displays" can be empty.... And return only "uid" and "guid" (mandatory fields) as sessions from sql_auth...


Sat, 29 Apr 2017 06:59:30 GMT - Denis01:

So should be like that (4 version in attached file)


Sat, 29 Apr 2017 07:00:01 GMT - Denis01: attachment set


Sat, 29 Apr 2017 07:08:02 GMT - Denis01:

And 5th version. DB schema adjusted - "username", "uid", "guid" have to be UNIQUE and NOT NULL


Sat, 29 Apr 2017 07:08:20 GMT - Denis01: attachment set


Sat, 29 Apr 2017 08:56:26 GMT - Denis01:

No,uid and guid should not be UNIQUE. Users can login to the same environement Will be corrected in the code


Sat, 29 Apr 2017 10:25:21 GMT - Antoine Martin:

Merged with the following changes in r15752 (+ r15756 fixup for MS Windows):

Tested by hand using:

python ./xpra/server/auth/sqlite_auth.py auth.sdb create
python ./xpra/server/auth/sqlite_auth.py auth.sdb add foo bar
xpra start --bind-tcp=0.0.0.0: --tcp-auth=sqlite:filename=`pwd`/auth.sdb
xpra attach tcp/foo:bar@localhost/

@Denis01: please close if this works for you.


Sat, 29 Apr 2017 11:42:55 GMT - Denis01: status changed; resolution set

Superb! Works. tested with

xpra proxy ....

Sat, 29 Apr 2017 11:51:33 GMT - Antoine Martin:

Minor fix: r15757 allows us to use relative paths, even when daemonizing the server.


Wed, 24 May 2017 07:12:16 GMT - Antoine Martin: milestone changed

(edit milestone)


Fri, 14 Jul 2017 17:56:28 GMT - Antoine Martin: summary changed

(editing ticket title)


Thu, 20 Jul 2017 13:32:06 GMT - Antoine Martin: summary changed


Thu, 16 Nov 2017 04:38:12 GMT - Antoine Martin:

As of r17435, the username is no longer unique and we allow multiple passwords for the same user.


Sat, 22 Sep 2018 15:18:54 GMT - Antoine Martin:

r17435 caused a bug: since the username is no longer unique (only username+password is), the query for returning the sessions needed updating to take into account the password (which we need to keep around for that purpose). Done in r20501.


Sun, 05 May 2019 02:49:50 GMT - Antoine Martin:

Follow up: #2287 for mysql.


Sat, 23 Jan 2021 05:25:40 GMT - migration script:

this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/1488