Implement phone number normalization and access control for Telegram users
- Added functionality to normalize phone numbers for comparison, ensuring only digits are stored and checked. - Updated configuration to include optional phone number allowlists for users and admins in the environment settings. - Enhanced authentication logic to allow access based on normalized phone numbers, in addition to usernames. - Introduced new helper functions for parsing and validating phone numbers, improving code organization and maintainability. - Added unit tests to validate phone normalization and access control based on phone numbers.
This commit is contained in:
@@ -55,8 +55,9 @@ def test_duties_200_when_skip_auth(mock_fetch, client):
|
||||
|
||||
|
||||
@patch("duty_teller.api.dependencies.validate_init_data_with_reason")
|
||||
@patch("duty_teller.api.dependencies.config.MINI_APP_SKIP_AUTH", False)
|
||||
def test_duties_403_when_init_data_invalid(mock_validate, client):
|
||||
mock_validate.return_value = (None, "hash_mismatch", "en")
|
||||
mock_validate.return_value = (None, None, "hash_mismatch", "en")
|
||||
r = client.get(
|
||||
"/api/duties",
|
||||
params={"from": "2025-01-01", "to": "2025-01-31"},
|
||||
@@ -72,11 +73,16 @@ def test_duties_403_when_init_data_invalid(mock_validate, client):
|
||||
)
|
||||
|
||||
|
||||
@patch("duty_teller.api.dependencies.get_user_by_telegram_id")
|
||||
@patch("duty_teller.api.dependencies.validate_init_data_with_reason")
|
||||
@patch("duty_teller.api.dependencies.config.can_access_miniapp")
|
||||
def test_duties_403_when_username_not_allowed(mock_can_access, mock_validate, client):
|
||||
mock_validate.return_value = ("someuser", "ok", "en")
|
||||
@patch("duty_teller.api.dependencies.config.MINI_APP_SKIP_AUTH", False)
|
||||
def test_duties_403_when_username_not_allowed(
|
||||
mock_can_access, mock_validate, mock_get_user, client
|
||||
):
|
||||
mock_validate.return_value = (123, "someuser", "ok", "en")
|
||||
mock_can_access.return_value = False
|
||||
mock_get_user.return_value = None # no user in DB or no phone path
|
||||
with patch("duty_teller.api.app.fetch_duties_response") as mock_fetch:
|
||||
r = client.get(
|
||||
"/api/duties",
|
||||
@@ -90,10 +96,87 @@ def test_duties_403_when_username_not_allowed(mock_can_access, mock_validate, cl
|
||||
mock_fetch.assert_not_called()
|
||||
|
||||
|
||||
@patch("duty_teller.api.dependencies.config.can_access_miniapp_by_phone")
|
||||
@patch("duty_teller.api.dependencies.get_user_by_telegram_id")
|
||||
@patch("duty_teller.api.dependencies.validate_init_data_with_reason")
|
||||
@patch("duty_teller.api.dependencies.config.can_access_miniapp")
|
||||
@patch("duty_teller.api.dependencies.config.MINI_APP_SKIP_AUTH", False)
|
||||
def test_duties_403_when_no_username_and_phone_not_in_allowlist(
|
||||
mock_can_access, mock_validate, mock_get_user, mock_can_access_phone, client
|
||||
):
|
||||
"""No username in initData and user's phone not in ALLOWED_PHONES -> 403."""
|
||||
from types import SimpleNamespace
|
||||
|
||||
mock_validate.return_value = (456, None, "ok", "en")
|
||||
mock_can_access.return_value = False
|
||||
mock_get_user.return_value = SimpleNamespace(phone="+79001111111", full_name="User")
|
||||
mock_can_access_phone.return_value = False
|
||||
with patch("duty_teller.api.app.fetch_duties_response") as mock_fetch:
|
||||
r = client.get(
|
||||
"/api/duties",
|
||||
params={"from": "2025-01-01", "to": "2025-01-31"},
|
||||
headers={"X-Telegram-Init-Data": "user=xxx&hash=yyy"},
|
||||
)
|
||||
assert r.status_code == 403
|
||||
mock_fetch.assert_not_called()
|
||||
|
||||
|
||||
@patch("duty_teller.api.dependencies.config.can_access_miniapp_by_phone")
|
||||
@patch("duty_teller.api.dependencies.get_user_by_telegram_id")
|
||||
@patch("duty_teller.api.dependencies.validate_init_data_with_reason")
|
||||
@patch("duty_teller.api.dependencies.config.can_access_miniapp")
|
||||
@patch("duty_teller.api.dependencies.config.MINI_APP_SKIP_AUTH", False)
|
||||
def test_duties_200_when_no_username_but_phone_in_allowlist(
|
||||
mock_can_access, mock_validate, mock_get_user, mock_can_access_phone, client
|
||||
):
|
||||
"""No username in initData but user's phone in ALLOWED_PHONES -> 200."""
|
||||
from types import SimpleNamespace
|
||||
|
||||
mock_validate.return_value = (789, None, "ok", "en")
|
||||
mock_can_access.return_value = False
|
||||
mock_get_user.return_value = SimpleNamespace(
|
||||
phone="+7 900 123-45-67", full_name="Иван"
|
||||
)
|
||||
mock_can_access_phone.return_value = True
|
||||
with patch("duty_teller.api.app.fetch_duties_response") as mock_fetch:
|
||||
mock_fetch.return_value = []
|
||||
r = client.get(
|
||||
"/api/duties",
|
||||
params={"from": "2025-01-01", "to": "2025-01-31"},
|
||||
headers={"X-Telegram-Init-Data": "user=xxx&hash=yyy"},
|
||||
)
|
||||
assert r.status_code == 200
|
||||
assert r.json() == []
|
||||
mock_fetch.assert_called_once_with(ANY, "2025-01-01", "2025-01-31")
|
||||
|
||||
|
||||
@patch("duty_teller.api.dependencies.validate_init_data_with_reason")
|
||||
@patch("duty_teller.api.dependencies.config.can_access_miniapp")
|
||||
@patch("duty_teller.api.dependencies.config.MINI_APP_SKIP_AUTH", True)
|
||||
def test_duties_200_with_valid_init_data_and_skip_auth_not_in_allowlist(
|
||||
mock_can_access, mock_validate, client
|
||||
):
|
||||
"""With MINI_APP_SKIP_AUTH=True, valid initData grants access without allowlist check."""
|
||||
mock_validate.return_value = ("outsider_user", "ok", "en")
|
||||
mock_can_access.return_value = False
|
||||
with patch("duty_teller.api.app.fetch_duties_response") as mock_fetch:
|
||||
mock_fetch.return_value = []
|
||||
r = client.get(
|
||||
"/api/duties",
|
||||
params={"from": "2025-01-01", "to": "2025-01-31"},
|
||||
headers={"X-Telegram-Init-Data": "user=xxx&hash=yyy"},
|
||||
)
|
||||
assert r.status_code == 200
|
||||
assert r.json() == []
|
||||
mock_fetch.assert_called_once_with(ANY, "2025-01-01", "2025-01-31")
|
||||
mock_can_access.assert_not_called()
|
||||
mock_validate.assert_not_called()
|
||||
|
||||
|
||||
@patch("duty_teller.api.dependencies.validate_init_data_with_reason")
|
||||
@patch("duty_teller.api.dependencies.config.can_access_miniapp")
|
||||
def test_duties_200_with_allowed_user(mock_can_access, mock_validate, client):
|
||||
mock_validate.return_value = ("alloweduser", "ok", "en")
|
||||
mock_validate.return_value = (1, "alloweduser", "ok", "en")
|
||||
mock_can_access.return_value = True
|
||||
with patch("duty_teller.api.app.fetch_duties_response") as mock_fetch:
|
||||
mock_fetch.return_value = [
|
||||
|
||||
@@ -34,3 +34,36 @@ def test_can_access_miniapp_denied(monkeypatch):
|
||||
monkeypatch.setattr(config, "ADMIN_USERNAMES", set())
|
||||
assert config.can_access_miniapp("other") is False
|
||||
assert config.can_access_miniapp("") is False
|
||||
|
||||
|
||||
def test_normalize_phone():
|
||||
"""Phone is normalized to digits only."""
|
||||
assert config.normalize_phone("+7 900 123-45-67") == "79001234567"
|
||||
assert config.normalize_phone("8 (900) 123-45-67") == "89001234567"
|
||||
assert config.normalize_phone("79001234567") == "79001234567"
|
||||
assert config.normalize_phone("") == ""
|
||||
assert config.normalize_phone(None) == ""
|
||||
|
||||
|
||||
def test_can_access_miniapp_by_phone(monkeypatch):
|
||||
monkeypatch.setattr(config, "ALLOWED_PHONES", {"79001234567", "79007654321"})
|
||||
monkeypatch.setattr(config, "ADMIN_PHONES", set())
|
||||
assert config.can_access_miniapp_by_phone("+7 900 123-45-67") is True
|
||||
assert config.can_access_miniapp_by_phone("79007654321") is True
|
||||
assert config.can_access_miniapp_by_phone("79001111111") is False
|
||||
assert config.can_access_miniapp_by_phone(None) is False
|
||||
assert config.can_access_miniapp_by_phone("") is False
|
||||
|
||||
|
||||
def test_can_access_miniapp_by_phone_admin(monkeypatch):
|
||||
monkeypatch.setattr(config, "ALLOWED_PHONES", set())
|
||||
monkeypatch.setattr(config, "ADMIN_PHONES", {"79001111111"})
|
||||
assert config.can_access_miniapp_by_phone("+7 900 111-11-11") is True
|
||||
assert config.can_access_miniapp_by_phone("79002222222") is False
|
||||
|
||||
|
||||
def test_is_admin_by_phone(monkeypatch):
|
||||
monkeypatch.setattr(config, "ADMIN_PHONES", {"79001111111"})
|
||||
assert config.is_admin_by_phone("+7 900 111-11-11") is True
|
||||
assert config.is_admin_by_phone("79001234567") is False
|
||||
assert config.is_admin_by_phone(None) is False
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
"""Tests for duty_teller.api.telegram_auth.validate_init_data."""
|
||||
"""Tests for duty_teller.api.telegram_auth.validate_init_data and validate_init_data_with_reason."""
|
||||
|
||||
from duty_teller.api.telegram_auth import validate_init_data
|
||||
from duty_teller.api.telegram_auth import (
|
||||
validate_init_data,
|
||||
validate_init_data_with_reason,
|
||||
)
|
||||
from tests.helpers import make_init_data
|
||||
|
||||
|
||||
@@ -40,13 +43,42 @@ def test_missing_user_returns_none():
|
||||
assert validate_init_data(init_data, bot_token) is None
|
||||
|
||||
|
||||
def test_user_without_username_returns_none():
|
||||
def test_user_without_username_returns_none_from_validate_init_data():
|
||||
"""validate_init_data returns None when user has no username (backward compat)."""
|
||||
bot_token = "123:ABC"
|
||||
user = {"id": 123, "first_name": "Test"} # no username
|
||||
init_data = make_init_data(user, bot_token)
|
||||
assert validate_init_data(init_data, bot_token) is None
|
||||
|
||||
|
||||
def test_user_without_username_but_with_id_succeeds_with_reason():
|
||||
"""With validate_init_data_with_reason, valid user.id is enough; username may be None."""
|
||||
bot_token = "123:ABC"
|
||||
user = {"id": 456, "first_name": "Test", "language_code": "ru"}
|
||||
init_data = make_init_data(user, bot_token)
|
||||
telegram_user_id, username, reason, lang = validate_init_data_with_reason(
|
||||
init_data, bot_token
|
||||
)
|
||||
assert telegram_user_id == 456
|
||||
assert username is None
|
||||
assert reason == "ok"
|
||||
assert lang == "ru"
|
||||
|
||||
|
||||
def test_user_without_id_returns_no_user_id():
|
||||
"""When user object exists but has no 'id', return no_user_id."""
|
||||
bot_token = "123:ABC"
|
||||
user = {"first_name": "Test"} # no id
|
||||
init_data = make_init_data(user, bot_token)
|
||||
telegram_user_id, username, reason, lang = validate_init_data_with_reason(
|
||||
init_data, bot_token
|
||||
)
|
||||
assert telegram_user_id is None
|
||||
assert username is None
|
||||
assert reason == "no_user_id"
|
||||
assert lang == "en"
|
||||
|
||||
|
||||
def test_empty_init_data_returns_none():
|
||||
assert validate_init_data("", "token") is None
|
||||
assert validate_init_data(" ", "token") is None
|
||||
|
||||
Reference in New Issue
Block a user