Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connect to multiple School Server easily #679

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
186 changes: 137 additions & 49 deletions src/jarabe/desktop/schoolserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@
import time
import uuid
import sys
import urllib2
import re
import json
import subprocess

from gi.repository import Gio

Expand All @@ -33,6 +37,7 @@
from sugar3.profile import get_nick_name

_REGISTER_URL = 'http://schoolserver:8080/'
_SERVER_DB_URL = 'http://schoolserver:5000/'
_REGISTER_TIMEOUT = 8
_OFW_TREE = '/ofw'
_PROC_TREE = '/proc/device-tree'
Expand All @@ -56,32 +61,62 @@ def _generate_serial_number():
return serial


def _store_identifiers(serial_number, uuid_, backup_url):
""" Stores the serial number, uuid and backup_url
in the identifier folder inside the profile directory
so that these identifiers can be used for backup. """

def _get_identifier_path():
identifier_path = os.path.join(env.get_profile_path(), 'identifiers')
if not os.path.exists(identifier_path):
os.mkdir(identifier_path)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These three lines are duplicate of code in _store_identifiers, please refactor.

return identifier_path


def _write_to_file(path, value, mode='w+'):
file_handler = open(path, mode)
file_handler.write(value)
file_handler.close()


def _read_from_file(path):
file_handler = open(path, 'r')
data = file_handler.read()
file_handler.close()
return data


def _get_history_for_serial(serial_number):
identifier_path = _get_identifier_path()
file_path = os.path.join(identifier_path, 'server_history')
if os.path.exists(file_path):
data = json.loads(_read_from_file(file_path))
if serial_number in data:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lines (join a path, check exists, open, load, close) are also closely related to the other lines you add below, please refactor into a read from a file in identifiers path, returning data.

return (True, data[serial_number])
else:
return (False, None)
else:
return (False, None)


if os.path.exists(os.path.join(identifier_path, 'sn')):
os.remove(os.path.join(identifier_path, 'sn'))
serial_file = open(os.path.join(identifier_path, 'sn'), 'w')
serial_file.write(serial_number)
serial_file.close()
def _store_identifiers(serial_number, uuid_, jabber_server, backup_url):
""" Stores the serial number, uuid and backup_url
in the identifier folder inside the profile directory
so that these identifiers can be used for backup. """

if os.path.exists(os.path.join(identifier_path, 'uuid')):
os.remove(os.path.join(identifier_path, 'uuid'))
uuid_file = open(os.path.join(identifier_path, 'uuid'), 'w')
uuid_file.write(uuid_)
uuid_file.close()
identifier_path = _get_identifier_path()

if os.path.exists(os.path.join(identifier_path, 'backup_url')):
os.remove(os.path.join(identifier_path, 'backup_url'))
backup_url_file = open(os.path.join(identifier_path, 'backup_url'), 'w')
backup_url_file.write(backup_url)
backup_url_file.close()
server_history = None
file_path = os.path.join(identifier_path, 'server_history')
if os.path.exists(file_path):
server_history = json.loads(_read_from_file(file_path))
else:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These three lines are identical to the last three lines on the else branch of the if, please move them out of the conditional clauses.

server_history = {}
server_history[serial_number] = {}
server_history[serial_number]["uuid"] = uuid_
server_history[serial_number]["backup_url"] = backup_url

values_to_store = {'sn': serial_number,
'uuid': uuid_,
'backup_url': jabber_server,
'server_history': json.dumps(server_history)}
for key in values_to_store:
_write_to_file(os.path.join(identifier_path, key), values_to_store[key])


class RegisterError(Exception):
Expand All @@ -107,10 +142,24 @@ def make_connection(self, host):
return _TimeoutHTTP(host, timeout=_REGISTER_TIMEOUT)


def register_laptop(url=_REGISTER_URL):
def register_laptop(url=_REGISTER_URL, db_url=_SERVER_DB_URL):
""" Registers the laptop to schoolserver. The process
can be devided into 4 parts.
1) Gather sn, uuid, nick, url and db_url.
2) If the laptop is pre-registered to this server than get the
sn, uuid_, backup_url and jabber_server info of the previous
registration.
3) Else, do fresh registration to the server and get new backup_url
and from the server.
4) Store sn, uuid_, backup_url and jabber_server information
for the present server at their setting locations. """

profile = get_profile()
new_registration_required = True
backup_url = ''
server_html = ''

# Gather data for registration (Part 1)
if _have_ofw_tree():
sn = _read_mfg_data(os.path.join(_OFW_TREE, _MFG_SN))
uuid_ = _read_mfg_data(os.path.join(_OFW_TREE, _MFG_UUID))
Expand All @@ -122,44 +171,85 @@ def register_laptop(url=_REGISTER_URL):
uuid_ = str(uuid.uuid1())
sn = sn or 'SHF00000000'
uuid_ = uuid_ or '00000000-0000-0000-0000-000000000000'

nick = get_nick_name()

settings = Gio.Settings('org.sugarlabs.collaboration')
jabber_server = settings.get_string('jabber-server')
_store_identifiers(sn, uuid_, jabber_server)

# Check server for registration
if jabber_server:
db_url = 'http://' + jabber_server + ':5000/'
url = 'http://' + jabber_server + ':8080/'

if sys.hexversion < 0x2070000:
server = xmlrpclib.ServerProxy(url, _TimeoutTransport())
else:
socket.setdefaulttimeout(_REGISTER_TIMEOUT)
server = xmlrpclib.ServerProxy(url)
try:
data = server.register(sn, nick, uuid_, profile.pubkey)
except (xmlrpclib.Error, TypeError, socket.error):
logging.exception('Registration: cannot connect to server')
raise RegisterError(_('Cannot connect to the server.'))
finally:
socket.setdefaulttimeout(None)

if data['success'] != 'OK':
logging.error('Registration: server could not complete request: %s',
data['error'])
raise RegisterError(_('The server could not complete the request.'))

settings.set_string('jabber-server', data['jabberserver'])
response = urllib2.urlopen(db_url)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use libsoup for opening urls. Libsoup properly intergrates with the gobject mainloop; so that the ui is still responsive while we wait on the network.

Lib soup has documentation, but you can probs just copy some code from my reddit client; https://github.com/samdroid-apps/something-for-reddit/blob/master/src/api.py#L182

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samdroid-apps Please review

server_html = response.read()
except (urllib2.URLError, urllib2.HTTPError):
logging.exception('Registration: cannot connect to xs-authserver')

# Restore previous registration (Part 2)
if server_html and profile.pubkey in server_html:
new_registration_required = False
registered_laptops = re.findall(r'{.+}', server_html)
for laptop in registered_laptops:
if profile.pubkey in laptop:
string_for_json = laptop.replace("&#39;", "\"").replace(
" u\"", "\"")
data = json.loads(string_for_json)
history_found, data_ = _get_history_for_serial(data["serial"])
new_registration_required = not history_found
if history_found:
uuid_ = data_["uuid"]
sn = data["serial"]
backup_url = data_["backup_url"]
try:
jabber_server = re.search(r'\@(.*)\:',
backup_url).group(1)
except AttributeError:
pass

# Do fresh registration (Part 3)
if new_registration_required:
if sys.hexversion < 0x2070000:
server = xmlrpclib.ServerProxy(url, _TimeoutTransport())
else:
socket.setdefaulttimeout(_REGISTER_TIMEOUT)
server = xmlrpclib.ServerProxy(url)
try:
data = server.register(sn, nick, uuid_, profile.pubkey)
except (xmlrpclib.Error, TypeError, socket.error):
logging.exception('Registration: cannot connect to server')
raise RegisterError(_('Cannot connect to the server.'))
finally:
socket.setdefaulttimeout(None)

if data['success'] != 'OK':
logging.error('Registration: server could not complete request: %s',
data['error'])
raise RegisterError(_('The server could not complete the request.'))

# Registration Successful, hence we can add the identification of this
# server to our known_host file.
command = "ssh-keyscan -H -t ecdsa " + jabber_server
output = subprocess.check_output(command, shell=True)
os.system("mkdir -p ~/.ssh")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • there's no need to assume that ~ may be missing (that is, don't use -p),
  • python can call mkdir directly,
  • the mask is not set, which may break later inbound SSH on this system,
  • there's no checking of status returned by these steps,
  • it isn't clear that an echo to known_hosts is safe; is there a program provided by OpenSSH that provides interlocked access?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • .ssh directory may be missing. Hence I used -p.
  • ya, i used os.system("mkdir -p ~/.ssh") instead of
    if not os.path.exists(...): os.makedirs(...)
    for its simplicity. Shall i use the later?
  • I didn't get the it. How would it break inbound SSH into the system?
  • while calling ssh-keyscan we already know that the server is present and jabber_server value has correct address due to successful registration, I think assuming and checking for error will be redundant. The output of the command if executed successfully will either be the identity key or blank. that is why I didn't check it before, but now have added a condition to write only if output has value.
  • I am not aware of the security of echoing to known_hosts. I replaced it with _write_to_file with append mode

if output:
_write_to_file("~/.ssh/known_hosts", output, 'a+')

jabber_server = data['jabberserver']
backup_url = data['backupurl']

# Store registration details (Part 4)
_store_identifiers(sn, uuid_, jabber_server, backup_url)
settings.set_string('jabber-server', jabber_server)
settings = Gio.Settings('org.sugarlabs')
settings.set_string('backup-url', data['backupurl'])
settings.set_string('backup-url', backup_url)

# DEPRECATED
from gi.repository import GConf
client = GConf.Client.get_default()
client.set_string(
'/desktop/sugar/collaboration/jabber_server', data['jabberserver'])
client.set_string('/desktop/sugar/backup_url', data['backupurl'])
'/desktop/sugar/collaboration/jabber_server', jabber_server)
client.set_string('/desktop/sugar/backup_url', backup_url)
client.set_string('/desktop/sugar/soas_serial', sn)

return True

Expand All @@ -175,7 +265,5 @@ def _have_proc_device_tree():
def _read_mfg_data(path):
if not os.path.exists(path):
return None
fh = open(path, 'r')
data = fh.read().rstrip('\0\n')
fh.close()
data = _read_from_file(path).rstrip('\0\n')
return data