Merge branch 'develop' into bump_python3.11

model/auth.py

@@ -116,7 +116,7 @@ def login(username, password):
             - 403 Login Failed
         '''
         try:
-            user = User.login(username, password)
+            user = User.login(username, password, request.remote_addr)
         except DoesNotExist:
             return HTTPError('Login Failed', 403)
         if not user.active:
@@ -152,7 +152,7 @@ def signup(username, password, email):
 @Request.json('old_password: str', 'new_password: str')
 def change_password(user, old_password, new_password):
     try:
-        User.login(user.username, old_password)
+        User.login(user.username, old_password, request.remote_addr)
     except DoesNotExist:
         return HTTPError('Wrong Password', 403)
     user.change_password(new_password)

mongo/engine.py

@@ -460,3 +460,10 @@ class SubmissionConfig(Config):
         ],
         db_field='sandboxInstances',
     )
+
+
+class LoginRecords(Document):
+    user_id = StringField(required=True)
+    ip_addr = StringField(required=True)
+    success = BooleanField(required=True, default=False)
+    timestamp = DateTimeField(required=True, default=datetime.now)

mongo/user.py

@@ -92,7 +92,7 @@ def batch_signup(
                 except engine.DoesNotExist:
                     new_user = cls.get_by_email(u['email'])
                 if force:
-                    new_user.force_update(u)
+                    new_user.force_update(u, course)
             registered_users.append(new_user)
         if course is not None:
             new_student_nicknames = {
@@ -103,7 +103,7 @@ def batch_signup(
             course.update_student_namelist(new_student_nicknames)
         return new_users
 
-    def force_update(self, new_user: Dict[str, Any]):
+    def force_update(self, new_user: Dict[str, Any], course: Optional[Course]):
         '''
         Force update an existent user in batch update procedure
         '''
@@ -113,18 +113,33 @@ def force_update(self, new_user: Dict[str, Any]):
             self.update(role=role)
         if (password := new_user.get('password')) is not None:
             self.change_password(password)
+        if (email := new_user.get('email')) is not None:
+            self.update(email=email,
+                        md5=hashlib.md5(email.encode()).hexdigest())
+        if course is not None:
+            self.update(add_to_set__courses=course.id)
         self.reload()
 
     @classmethod
-    def login(cls, username, password):
+    def login(cls, username, password, ip_addr):
         try:
             user = cls.get_by_username(username)
         except engine.DoesNotExist:
             user = cls.get_by_email(username)
         user_id = hash_id(user.username, password)
         if (compare_digest(user.user_id, user_id)
                 or compare_digest(user.user_id2, user_id)):
+            engine.LoginRecords(
+                user_id=user.id,
+                ip_addr=ip_addr,
+                success=True,
+            ).save(force_insert=True)
             return user
+        engine.LoginRecords(
+            user_id=user.id,
+            ip_addr=ip_addr,
+            success=False,
+        ).save(force_insert=True)
         raise engine.DoesNotExist
 
     @classmethod

tests/test_auth.py

@@ -633,7 +633,7 @@ def cmp_payload_and_user(
             assert user.profile.displayed_name == payload.displayed_name
         if payload.role is not None:
             assert user.role == payload.role
-        login = User.login(payload.username, payload.password)
+        login = User.login(payload.username, payload.password, "127.0.0.1")
         assert login.username == payload.username
 
     @classmethod
@@ -677,7 +677,7 @@ def test_normally_register(self, forge_client):
         assert rv.status_code == 200, rv.get_json()
         # Ensure the users has been registered
         for u in excepted_users:
-            login = User.login(u.username, u.password)
+            login = User.login(u.username, u.password, "127.0.0.1")
             assert login == User.get_by_username(u.username)
 
     def test_sign_up_with_invalid_csv(self, monkeypatch, forge_client):
@@ -738,7 +738,7 @@ def test_signup_with_existent_user(self, forge_client):
         assert rv.status_code == 200, rv.get_json()
         course = Course(course_name)
         for u in excepted_users:
-            login = User.login(u.username, u.password)
+            login = User.login(u.username, u.password, "127.0.0.1")
             assert login == User.get_by_username(u.username)
             assert u.username in course.student_nicknames
 
@@ -755,7 +755,7 @@ def test_signup_with_displayed_name(self, forge_client):
         )
         assert rv.status_code == 200, rv.get_json()
         for u in excepted_users:
-            login = User.login(u.username, u.password)
+            login = User.login(u.username, u.password, "127.0.0.1")
             assert login == User.get_by_username(u.username)
             assert login.profile.displayed_name == u.displayed_name
 
@@ -772,7 +772,7 @@ def test_signup_with_role(self, forge_client):
         )
         assert rv.status_code == 200, rv.get_json()
         for u in excepted_users:
-            login = User.login(u.username, u.password)
+            login = User.login(u.username, u.password, "127.0.0.1")
             assert login == User.get_by_username(u.username)
             assert login.role == u.role
 
@@ -786,7 +786,8 @@ def test_signup_without_optional_field(self, forge_client):
             },
         )
         assert rv.status_code == 200, rv.get_json()
-        login = User.login(except_user.username, except_user.password)
+        login = User.login(except_user.username, except_user.password,
+                           "127.0.0.1")
         assert login == User.get_by_username(except_user.username)
 
     def test_signup_with_invalid_input_format(self, forge_client):
@@ -848,9 +849,9 @@ def test_force_signup_should_override_existent_users(
         # ensure they can't login with updated payload
         for u in existent_users:
             with pytest.raises(engine.DoesNotExist):
-                User.login(u.username, u.password)
+                User.login(u.username, u.password, "127.0.0.1")
             with pytest.raises(engine.DoesNotExist):
-                User.login(u.email, u.password)
+                User.login(u.email, u.password, "127.0.0.1")
 
         excepted_users = [
             *(self.signup_input() for _ in range(5)),
@@ -871,7 +872,7 @@ def test_force_signup_should_override_existent_users(
 
         course.reload()
         for u in excepted_users:
-            login = User.login(u.username, u.password)
+            login = User.login(u.username, u.password, "127.0.0.1")
             self.cmp_payload_and_user(login, u)
             assert u.username in course.student_nicknames
 
@@ -901,3 +902,35 @@ def test_verify_link_with_subdirectory(app):
     expected_url = f'https://{server_name}{subdirectory}/auth/active/{u.cookie}'
     with app.app_context():
         assert expected_url == get_verify_link(u)
+
+
+def test_login_recorded_after_login(client):
+    password = 'pass'
+    u = utils.user.create_user(password=password)
+    resp = client.post(
+        '/auth/session',
+        json={
+            'username': u.username,
+            'password': password,
+        },
+    )
+    assert resp.status_code == 200
+
+    record = engine.LoginRecords.objects(user_id=u.id)
+    assert len(record) == 1
+
+
+def test_login_recorded_after_failed_login(client):
+    u = utils.user.create_user()
+    password = secrets.token_hex()
+    resp = client.post(
+        '/auth/session',
+        json={
+            'username': u.username,
+            'password': password,
+        },
+    )
+    assert resp.status_code == 403
+
+    record = engine.LoginRecords.objects(user_id=u.id, success=False)
+    assert len(record) == 1

tests/test_user.py

@@ -42,7 +42,7 @@ def test_admin_can_create_user(forge_client):
     )
     assert rv.status_code == 200, rv_json
 
-    u = User.login(payload.username, payload.password)
+    u = User.login(payload.username, payload.password, "127.0.0.1")
     assert u.email == payload.email
 
 
@@ -96,7 +96,7 @@ def test_non_admin_cannot_create_user(forge_client, role: engine.User.Role):
     )
     assert rv.status_code == 403, rv_json
     with pytest.raises(engine.DoesNotExist):
-        User.login(payload.username, payload.password)
+        User.login(payload.username, payload.password, "127.0.0.1")
 
 
 def test_admin_can_read_user_list(forge_client):
@@ -207,7 +207,8 @@ def test_admin_can_read_user_under_specific_course(forge_client):
 def test_admin_can_update_user_password(forge_client):
     password = secrets.token_hex()
     user = utils.user.create_user(password=password)
-    assert User.login(user.username, password).username == user.username
+    assert User.login(user.username, password,
+                      "127.0.0.1").username == user.username
 
     client = forge_client('first_admin')
     new_password = password + secrets.token_hex(4)
@@ -222,9 +223,10 @@ def test_admin_can_update_user_password(forge_client):
 
     # can't login with old password
     with pytest.raises(engine.DoesNotExist):
-        User.login(user.username, password)
+        User.login(user.username, password, "127.0.0.1")
     # should use new one
-    assert User.login(user.username, new_password).username == user.username
+    assert User.login(user.username, new_password,
+                      "127.0.0.1").username == user.username
 
 
 def test_admin_can_update_user_display_name(forge_client):