diff --git a/supernote/server/services/user.py b/supernote/server/services/user.py index 34056af9..849e4035 100644 --- a/supernote/server/services/user.py +++ b/supernote/server/services/user.py @@ -219,20 +219,51 @@ async def verify_login_hash( ) -> bool: user = await self._get_user_do(account) if not user or not user.is_active: + logger.warning("verify_login_hash: user %s not found or inactive", account) return False stored_value = await self._coordination_service.get_value( f"challenge:{account}" ) if not stored_value: + logger.warning( + "verify_login_hash: no challenge found for %s (expired or never issued)", + account, + ) return False random_code, stored_timestamp = stored_value.split("|") - if stored_timestamp != timestamp: + # Accept timestamps within the challenge TTL to tolerate device clock skew. + # Exact equality breaks when the device sends its own local time rather than + # echoing the server's challenge timestamp. The random_code nonce + TTL already + # provides replay protection, so strict equality is not needed here. + try: + ts_diff = abs(int(stored_timestamp) - int(timestamp)) + if ts_diff > int(RANDOM_CODE_TTL.total_seconds() * 1000): + logger.warning( + "verify_login_hash: timestamp skew too large for %s " + "(stored=%s client=%s diff=%dms limit=%dms)", + account, + stored_timestamp, + timestamp, + ts_diff, + int(RANDOM_CODE_TTL.total_seconds() * 1000), + ) + return False + except ValueError: + logger.warning( + "verify_login_hash: non-numeric timestamp for %s (stored=%s client=%s)", + account, + stored_timestamp, + timestamp, + ) return False expected_hash = hash_with_salt(user.password_md5, random_code) - return expected_hash == client_hash + if expected_hash != client_hash: + logger.warning("verify_login_hash: hash mismatch for %s", account) + return False + return True async def login( self, diff --git a/tests/server/services/test_verify_login_hash.py b/tests/server/services/test_verify_login_hash.py new file mode 100644 index 00000000..72a58b90 --- /dev/null +++ b/tests/server/services/test_verify_login_hash.py @@ -0,0 +1,80 @@ +import hashlib + +import pytest + +from supernote.models.user import UserRegisterDTO +from supernote.server.services.user import RANDOM_CODE_TTL, UserService +from supernote.server.utils.hashing import hash_with_salt + + +@pytest.fixture +async def registered_user(user_service: UserService) -> str: + email = "hashtest@example.com" + pw_md5 = hashlib.md5(b"password").hexdigest() + await user_service.register(UserRegisterDTO(email=email, password=pw_md5)) + return email + + +async def test_verify_login_hash_exact_timestamp( + registered_user: str, user_service: UserService +) -> None: + code, ts = await user_service.generate_random_code(registered_user) + pw_md5 = hashlib.md5(b"password").hexdigest() + client_hash = hash_with_salt(pw_md5, code) + assert await user_service.verify_login_hash(registered_user, client_hash, ts) + + +async def test_verify_login_hash_small_clock_skew( + registered_user: str, user_service: UserService +) -> None: + code, ts = await user_service.generate_random_code(registered_user) + pw_md5 = hashlib.md5(b"password").hexdigest() + client_hash = hash_with_salt(pw_md5, code) + # Simulate device clock 30 seconds ahead of server + skewed_ts = str(int(ts) + 30_000) + assert await user_service.verify_login_hash(registered_user, client_hash, skewed_ts) + + +async def test_verify_login_hash_skew_exceeds_ttl( + registered_user: str, user_service: UserService +) -> None: + code, ts = await user_service.generate_random_code(registered_user) + pw_md5 = hashlib.md5(b"password").hexdigest() + client_hash = hash_with_salt(pw_md5, code) + # Simulate device clock 1 hour ahead — exceeds the 5-minute TTL window + skewed_ts = str(int(ts) + int(RANDOM_CODE_TTL.total_seconds() * 1000) + 1) + assert not await user_service.verify_login_hash( + registered_user, client_hash, skewed_ts + ) + + +async def test_verify_login_hash_non_numeric_timestamp( + registered_user: str, user_service: UserService +) -> None: + code, _ts = await user_service.generate_random_code(registered_user) + pw_md5 = hashlib.md5(b"password").hexdigest() + client_hash = hash_with_salt(pw_md5, code) + assert not await user_service.verify_login_hash( + registered_user, client_hash, "not-a-number" + ) + + +async def test_verify_login_hash_no_challenge( + registered_user: str, user_service: UserService +) -> None: + # Never call generate_random_code — no challenge stored + pw_md5 = hashlib.md5(b"password").hexdigest() + assert not await user_service.verify_login_hash(registered_user, pw_md5, "12345678") + + +async def test_verify_login_hash_wrong_hash( + registered_user: str, user_service: UserService +) -> None: + _code, ts = await user_service.generate_random_code(registered_user) + assert not await user_service.verify_login_hash(registered_user, "deadbeef" * 8, ts) + + +async def test_verify_login_hash_unknown_user(user_service: UserService) -> None: + assert not await user_service.verify_login_hash( + "nobody@example.com", "hash", "12345678" + )