Create possibility to keep the users records (like multifile records structure) in mySQL table
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.
stub for sql auth
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.
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.
program_context: don't worry about this, you don't need to understand it
it seems that Sqlite doesn't support sql schemes. So 2 ways are possible: create a DB in ":memory:" create a DB as a second db file
No idea what you mean here. All SQL databases support creating a schema for sure: https://sqlite.org/lang_createtable.html.
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
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.
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 :-)
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...
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.
Hello! I apologize! was very tired and posted another file! Hope that new one is better
sql_auth_NEW.py is a correct one
That's better.
Please address the following points:
init function is not needed, sqlauth does not use the legacy password file option
temp. probably to be modified for sql file location reading. Copied from Multifile - there is nothing to modify here, the filename is configurable, as it should be
self.db_name=filename -> self.db_name = filename
log.error("SQL_auth. No database: '%s'", self.db_name), please use the format: log.error("Error: sqlauth cannot open the database file '%s'", self.db_name) - also, this codepath should return "None" explicitly.
cursor.execute(self.password_query, ([self.username]))
self.sessions, use a "sessions" local variable instead
sqlite3.connect (ie: if the file is invalid), this should be handled
Now for the "create" command code:
print
sys.argv[x] repeatedly, assign it to a descriptive variable name
log.error("Error: failed to create database schema: %s", e)
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
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?
user_conn_str is not used - remove it, this doesn't look like the right place for it anyway
sql_file_name should be filename (makes it consistent options argument name, we know it is sql already anyway)
cursor = conn.cursor() should be moved in the try-except block as this may trigger exceptions
conn.commit() trigger exceptions? Is it even needed when doing reads?
entry=str(data[0]) is missing spaces, don't bother creating a temporary "entry" variable, just return the value (the same could be done for sys.exit + main)
self.uid=... etc, all the way to self.session_options: missing spaces, don't use self.
main - probably tabs vs spaces
if cmd=="create": code to a function, and move all the database access code within the try-except block
Other things left to be done:
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
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
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 :-)
Still many things to fix:
#import xpra_lb.py
init function
else:
pass
user_conn_str still needs removing, and the empty if statements that go with it
conn.close - doesn't call the function
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)
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()
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.
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.
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
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.
Here's a hint: address ALL my earlier comments and it will probably be called. Indentation matters.
:-) 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
Just without it can't remove "init" function. Filename is not defined
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.
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
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.
Ok, done. And all the comments from "comment:19" are included.
Looks good, only found two remaining problems:
get_sessions, the "sessions" variable may not be defined when reaching the end of that function
data[3] to decide to load the data or not. It would probably be clearer to assign those to meaningful variable names instead.
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).
:-)
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...
So should be like that (4 version in attached file)
And 5th version. DB schema adjusted - "username", "uid", "guid" have to be UNIQUE and NOT NULL
No,uid and guid should not be UNIQUE. Users can login to the same environement Will be corrected in the code
Merged with the following changes in r15752 (+ r15756 fixup for MS Windows):
CREATE TABLE
CREATE TABLE line into multiple lines
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.
Superb! Works. tested with
xpra proxy ....
Minor fix: r15757 allows us to use relative paths, even when daemonizing the server.
(edit milestone)
(editing ticket title)
As of r17435, the username is no longer unique and we allow multiple passwords for the same user.
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.
Follow up: #2287 for mysql.
this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/1488