- code reorganization in serve.py and app.py.

- login handler rewritten. Empty passwords are kept empty (ie, undefined). - fix: sqlalchemy use to work with Session instead of scoped_session, since tornado is single threaded. - fix: several helper functions were using the db_session contextmanager wrongly.

- code reorganization in serve.py and app.py.
- login handler rewritten. Empty passwords are kept empty (ie, undefined). - fix: sqlalchemy use to work with Session instead of scoped_session, since tornado is single threaded. - fix: several helper functions were using the db_session contextmanager wrongly.
Miguel Barão
1 parent d36a0c3e
Showing 2 changed files with 76 additions and 65 deletions Show diff stats
app.py
serve.py
@@ -7,7 +7,7 @@ from contextlib import contextmanager  # `with` statement in db sessions
 # user installed packages
 import bcrypt
 from sqlalchemy import create_engine
-from sqlalchemy.orm import sessionmaker, scoped_session
+from sqlalchemy.orm import sessionmaker #, scoped_session
 # this project
 from models import Student, Test, Question
@@ -21,19 +21,34 @@ class AppException(Exception):
     pass
 # ============================================================================
+# helper functions
+# ============================================================================
+def check_password(try_pw, password):
+    return password == bcrypt.hashpw(try_pw.encode('utf-8'), password)
+
+# ============================================================================
 #   Application
 # ============================================================================
 class App(object):
+    # -----------------------------------------------------------------------
+    # helper to manage db sessions using the `with` statement, for example
+    #   with self.db_session() as s:  s.query(...)
+    @contextmanager
+    def db_session(self):
+        session = self.Session()
+        try:
+            yield session
+            session.commit()
+        except:
+            session.rollback()
+            logger.error('DB rollback!!!')
+        finally:
+            session.close()
+
+    # ------------------------------------------------------------------------
     def __init__(self, conf={}):
-        # online = {
-        #   uid1: {
-        #     'student': {'number': 123, 'name': john, ...},
-        #     'test': {...}
-        #   },
-        #   uid2: {...}
-        # }
         logger.info('Starting application')
-        self.online = dict()    # {uid: {'student':{}}}
+        self.online = dict()    # {uid: {'student':{...}, 'test': {...}}, ...}
         self.allowed = set([])  # '0' is hardcoded to allowed elsewhere
         # build test configuration dictionary
@@ -54,7 +69,8 @@ class App(object):
         # connect to database and check registered students
         dbfile = path.expanduser(self.testfactory['database'])
         engine = create_engine(f'sqlite:///{dbfile}', echo=False)
-        self.Session = scoped_session(sessionmaker(bind=engine))  # FIXME not scoped in tornado
+        # self.Session = scoped_session(sessionmaker(bind=engine))  # FIXME not scoped in tornado
+        self.Session = sessionmaker(bind=engine)
         try:
             with self.db_session() as s:
@@ -79,51 +95,37 @@ class App(object):
         logger.critical('----------- !!! Server terminated !!! -----------')
     # -----------------------------------------------------------------------
-    # helper to manage db sessions using the `with` statement, for example
-    #   with self.db_session() as s:  s.query(...)
-    @contextmanager
-    def db_session(self):
-        try:
-            yield self.Session()
-        finally:
-            self.Session.remove()
-
-    # -----------------------------------------------------------------------
     def login(self, uid, try_pw):
-        if uid not in self.allowed and uid != '0':
-            # not allowed
+        if uid.startswith('l'):  # remove prefix 'l'
+            uid = uid[1:]
+
+        if uid not in self.allowed and uid != '0':      # not allowed
             logger.warning(f'Student {uid}: not allowed to login.')
             return False
+        # get name+password from db
         with self.db_session() as s:
-            student =  s.query(Student).filter(Student.id == uid).one_or_none()
-
-            if student is None:
-                # not found
-                logger.warning(f'Student {uid}: not found in database.')
-                return False
-
-            if student.password == '':
-                # update password on first login
-                hashed_pw = bcrypt.hashpw(try_pw.encode('utf-8'), bcrypt.gensalt())
-                student.password = hashed_pw
-                s.commit()
-                logger.info(f'Student {uid}: first login, password updated.')
-
-            elif bcrypt.hashpw(try_pw.encode('utf-8'), student.password) != student.password:
-                # wrong password
-                logger.info(f'Student {uid}: wrong password.')
-                return False
-
-            # success
-            self.allowed.discard(uid)
+            name, password = s.query(Student.name, Student.password).filter_by(id=uid).one()
+
+        # first login updates the password
+        if password == '':              # update password on first login
+            self.update_student_password(uid, try_pw)
+            pw_ok = True
+        else:                           # check password
+            pw_ok = check_password(try_pw, password)
+
+        if pw_ok:      # success            
+            self.allowed.discard(uid)  # remove from set of allowed students
             if uid in self.online:
                 logger.warning(f'Student {uid}: already logged in.')
-            else:
-                self.online[uid] = {'student': {'name': student.name, 'number': uid}}
+            else:                      # make student online
+                self.online[uid] = {'student': {'name': name, 'number': uid}}
                 logger.info(f'Student {uid}: logged in.')
+            return True
+        else:          # wrong password
+            logger.info(f'Student {uid}: wrong password.')
+            return False
-        return True
     # -----------------------------------------------------------------------
     def logout(self, uid):
@@ -174,7 +176,6 @@ class App(object):
                 finishtime=str(t['finish_time']),
                 student_id=t['student']['number'],
                 test_id=t['ref']) for q in t['questions'] if 'grade' in q])
-            s.commit()
         logger.info(f'Student {uid}: finished test.')
         return grade
@@ -203,7 +204,6 @@ class App(object):
                 student_id=t['student']['number'],
                 state=t['state'],
                 comment=''))
-            s.commit()
         logger.info(f'Student {uid}: gave up.')
         return t
@@ -219,8 +219,11 @@ class App(object):
         return self.testfactory['questions_dir']
     def get_student_grades_from_all_tests(self, uid):
         with self.db_session() as s:
-            r = s.query(Test).filter_by(student_id=uid).order_by(Test.finishtime).all()
-        return [(t.title, t.grade, t.finishtime) for t in r]
+            # r = s.query(Test).filter_by(student_id=uid).order_by(Test.finishtime).all()
+            # return [(t.title, t.grade, t.finishtime) for t in r]
+            print('here')
+            return s.query(Test.title, Test.grade, Test.finishtime).filter_by(student_id=uid).order_by(Test.finishtime).all()
+
     def get_json_filename_of_test(self, test_id):
         with self.db_session() as s:
             return s.query(Test.filename).filter_by(id=test_id).one_or_none()[0]
@@ -236,12 +239,14 @@ class App(object):
         # list of all ('uid', 'name', 'password') sorted by uid
         with self.db_session() as s:
             r = s.query(Student).all()
-        return sorted(((u.id, u.name, u.password) for u in r if u.id != '0'), key=lambda k: k[0])
+            return sorted(((u.id, u.name, u.password) for u in r if u.id != '0'), key=lambda k: k[0])
     def get_student_grades_from_test(self, uid, testid):
         with self.db_session() as s:
             r = s.query(Test).filter_by(student_id=uid).filter_by(ref=testid).all()
-        return [(u.grade, u.finishtime, u.id) for u in r]
+            return [(u.grade, u.finishtime, u.id) for u in r]
+
+
     def get_students_state(self):
         # [{
@@ -286,17 +291,18 @@ class App(object):
         self.allowed.discard(uid)
         logger.info(f'Student {uid}: denied to login')
-    def reset_password(self, uid):
+    def update_student_password(self, uid, pw=''):
+        if pw:
+            pw = bcrypt.hashpw(pw.encode('utf-8'), bcrypt.gensalt())
         with self.db_session() as s:
-            u = s.query(Student).filter(Student.id == uid).update({'password': ''})
-            s.commit()
-        logger.info(f'Student {uid}: password reset')
+            student = s.query(Student).filter_by(id=uid).one()
+            student.password = pw
+        logger.info(f'Student {uid}: password updated.')
     def insert_new_student(self, uid, name):
         try:
             with self.db_session() as s:
                 s.add(Student(id=uid, name=name, password=''))
-                s.commit()
         except Exception:
             logger.error(f'Insert failed: student {uid} already exists.')
         else:
@@ -31,7 +31,7 @@ class WebApplication(tornado.web.Application):
             (r'/test',          TestHandler),
             (r'/review',        ReviewHandler),
             (r'/admin',         AdminHandler),
-            (r'/file',      FileHandler), # FIXME
+            (r'/file',          FileHandler),
             (r'/',              RootHandler), # TODO multiple tests
         ]
@@ -50,7 +50,7 @@ class WebApplication(tornado.web.Application):
 # -------------------------------------------------------------------------
-# Base handler common to all handlers.
+# Base handler. Other handlers will inherit this one.
 # -------------------------------------------------------------------------
 class BaseHandler(tornado.web.RequestHandler):
     @property
@@ -64,17 +64,17 @@ class BaseHandler(tornado.web.RequestHandler):
 # -------------------------------------------------------------------------
-# /login  and  /logout
+# /login
 # -------------------------------------------------------------------------
 class LoginHandler(BaseHandler):
+    SUPPORTED_METHODS = ['GET', 'POST']
+
     def get(self):
         self.render('login.html', error='')
     # async 
     def post(self):
         uid = self.get_body_argument('uid')
-        if uid.startswith('l'):  # remove prefix 'l'
-            uid = uid[1:]
         pw = self.get_body_argument('pw')
         # loop = asyncio.get_event_loop()
@@ -88,7 +88,8 @@ class LoginHandler(BaseHandler):
             self.render("login.html",
                         error='Não autorizado ou número/senha inválido')
-
+# -------------------------------------------------------------------------
+# /logout
 # -------------------------------------------------------------------------
 class LogoutHandler(BaseHandler):
     @tornado.web.authenticated
@@ -113,6 +114,8 @@ class FileHandler(BaseHandler):
         ref = self.get_query_argument('ref', None)
         image = self.get_query_argument('image', None)
+        # FIXME does not work when user 0 is reviewing a test
+
         t = self.testapp.get_student_test(uid)
         if t is not None:
             for q in t['questions']:
@@ -277,10 +280,12 @@ class RootHandler(BaseHandler):
 # -------------------------------------------------------------------------
 class AdminHandler(BaseHandler):
+    SUPPORTED_METHODS = ['GET', 'POST']
+
     @tornado.web.authenticated
     def get(self):
         if self.current_user != '0':
-            raise tornado.web.HTTPError(404) # FIXME denied or not found??
+            raise tornado.web.HTTPError(403)
         cmd = self.get_query_argument('cmd', default=None)
@@ -318,7 +323,7 @@ class AdminHandler(BaseHandler):
             self.testapp.deny_student(value)
         elif cmd == 'reset_password':
-            self.testapp.reset_password(value)
+            self.testapp.update_student_password(uid=value, pw='')
         elif cmd == 'insert_student':
             s = json.loads(value)