Skip to content

Commit

Permalink
Merge pull request #289 from Maxxxxxx-x/feat/login-records
Browse files Browse the repository at this point in the history
feat/login records
  • Loading branch information
Bogay authored Nov 4, 2024
2 parents 2e3c556 + 0099542 commit 934d563
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 17 deletions.
4 changes: 2 additions & 2 deletions model/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions mongo/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
6 changes: 5 additions & 1 deletion mongo/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,19 @@ def force_update(self, new_user: Dict[str, Any], course: Optional[Course]):
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).save(force_insert=True)
raise engine.DoesNotExist

@classmethod
Expand Down
19 changes: 10 additions & 9 deletions tests/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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

Expand All @@ -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):
Expand Down Expand Up @@ -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)),
Expand All @@ -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

Expand Down
12 changes: 7 additions & 5 deletions tests/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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)
Expand All @@ -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):
Expand Down

0 comments on commit 934d563

Please sign in to comment.