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

feat/login records #289

Merged
merged 10 commits into from
Nov 4, 2024
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
Loading