feat: implement role-based access control for miniapp

- Introduced a new roles table in the database to manage user roles ('user' and 'admin') for access control.
- Updated the user model to include a foreign key reference to the roles table, allowing for role assignment.
- Enhanced command handlers to support the `/set_role` command for admins to assign roles to users.
- Refactored access control logic to utilize role checks instead of username/phone allowlists, improving security and maintainability.
- Updated documentation to reflect changes in access control mechanisms and role management.
- Added unit tests to ensure correct functionality of role assignment and access checks.

tests/test_app.py

@@ -82,14 +82,11 @@ 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")
 @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
-):
+def test_duties_403_when_user_not_in_db(mock_validate, mock_get_user, client):
+    """User not in DB -> 403 (access only for users with role or env-admin fallback)."""
     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
+    mock_get_user.return_value = None
     with patch("duty_teller.api.app.fetch_duties_response") as mock_fetch:
         r = client.get(
             "/api/duties",
@@ -103,21 +100,21 @@ def test_duties_403_when_username_not_allowed(
     mock_fetch.assert_not_called()
 
 
-@patch("duty_teller.api.dependencies.config.can_access_miniapp_by_phone")
+@patch("duty_teller.api.dependencies.can_access_miniapp_for_telegram_user")
 @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
+def test_duties_403_when_no_username_and_no_access(
+    mock_validate, mock_get_user, mock_can_access, client
 ):
-    """No username in initData and user's phone not in ALLOWED_PHONES -> 403."""
+    """User in DB but no miniapp access (no role, not env admin) -> 403."""
     from types import SimpleNamespace
 
     mock_validate.return_value = (456, None, "ok", "en")
+    mock_get_user.return_value = SimpleNamespace(
+        phone="+79001111111", full_name="User", username=None
+    )
     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",
@@ -128,23 +125,21 @@ def test_duties_403_when_no_username_and_phone_not_in_allowlist(
     mock_fetch.assert_not_called()
 
 
-@patch("duty_teller.api.dependencies.config.can_access_miniapp_by_phone")
+@patch("duty_teller.api.dependencies.can_access_miniapp_for_telegram_user")
 @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
+def test_duties_200_when_user_has_access(
+    mock_validate, mock_get_user, mock_can_access, client
 ):
-    """No username in initData but user's phone in ALLOWED_PHONES -> 200."""
+    """User in DB with miniapp access (role or env fallback) -> 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="Иван"
+        phone="+7 900 123-45-67", full_name="Иван", username=None
     )
-    mock_can_access_phone.return_value = True
+    mock_can_access.return_value = True
     with patch("duty_teller.api.app.fetch_duties_response") as mock_fetch:
         mock_fetch.return_value = []
         r = client.get(
@@ -180,10 +175,18 @@ def test_duties_200_with_valid_init_data_and_skip_auth_not_in_allowlist(
     mock_validate.assert_not_called()
 
 
+@patch("duty_teller.api.dependencies.can_access_miniapp_for_telegram_user")
+@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_200_with_allowed_user(mock_can_access, mock_validate, client):
+def test_duties_200_with_allowed_user(
+    mock_validate, mock_get_user, mock_can_access, client
+):
+    from types import SimpleNamespace
+
     mock_validate.return_value = (1, "alloweduser", "ok", "en")
+    mock_get_user.return_value = SimpleNamespace(
+        full_name="Иван Иванов", username="alloweduser", phone=None
+    )
     mock_can_access.return_value = True
     with patch("duty_teller.api.app.fetch_duties_response") as mock_fetch:
         mock_fetch.return_value = [
@@ -206,11 +209,16 @@ def test_duties_200_with_allowed_user(mock_can_access, mock_validate, client):
     mock_fetch.assert_called_once_with(ANY, "2025-01-01", "2025-01-31")
 
 
-def test_duties_e2e_auth_real_validation(client, monkeypatch):
+@patch("duty_teller.api.dependencies.can_access_miniapp_for_telegram_user")
+@patch("duty_teller.api.dependencies.get_user_by_telegram_id")
+def test_duties_e2e_auth_real_validation(
+    mock_get_user, mock_can_access, client, monkeypatch
+):
+    from types import SimpleNamespace
+
     test_token = "123:ABC"
     test_username = "e2euser"
     monkeypatch.setattr(config, "BOT_TOKEN", test_token)
-    monkeypatch.setattr(config, "ALLOWED_USERNAMES", {test_username})
     monkeypatch.setattr(config, "ADMIN_USERNAMES", set())
     monkeypatch.setattr(config, "INIT_DATA_MAX_AGE_SECONDS", 0)
     init_data = make_init_data(
@@ -218,6 +226,10 @@ def test_duties_e2e_auth_real_validation(client, monkeypatch):
         test_token,
         auth_date=int(time.time()),
     )
+    mock_get_user.return_value = SimpleNamespace(
+        full_name="E2E User", username=test_username, phone=None
+    )
+    mock_can_access.return_value = True
     with patch("duty_teller.api.app.fetch_duties_response") as mock_fetch:
         mock_fetch.return_value = []
         r = client.get(

tests/test_handlers_commands.py

@@ -175,24 +175,26 @@ async def test_calendar_link_with_user_and_token_replies_with_url():
             mock_user.id = 10
             mock_user.phone = None
             mock_get_user.return_value = mock_user
-            with patch("duty_teller.handlers.commands.config") as mock_cfg:
-                mock_cfg.can_access_miniapp.return_value = True
-                mock_cfg.can_access_miniapp_by_phone.return_value = False
-                mock_cfg.MINI_APP_BASE_URL = "https://example.com"
-                with patch(
-                    "duty_teller.handlers.commands.create_calendar_token",
-                    return_value="abc43token",
-                ):
+            with patch(
+                "duty_teller.handlers.commands.can_access_miniapp_for_telegram_user",
+                return_value=True,
+            ):
+                with patch("duty_teller.handlers.commands.config") as mock_cfg:
+                    mock_cfg.MINI_APP_BASE_URL = "https://example.com"
                     with patch(
-                        "duty_teller.handlers.commands.get_lang", return_value="en"
+                        "duty_teller.handlers.commands.create_calendar_token",
+                        return_value="abc43token",
                     ):
-                        with patch("duty_teller.handlers.commands.t") as mock_t:
-                            mock_t.side_effect = lambda lang, key, **kw: (
-                                f"URL: {kw.get('url', '')}"
-                                if "success" in key
-                                else "Hint"
-                            )
-                            await calendar_link(update, MagicMock())
+                        with patch(
+                            "duty_teller.handlers.commands.get_lang", return_value="en"
+                        ):
+                            with patch("duty_teller.handlers.commands.t") as mock_t:
+                                mock_t.side_effect = lambda lang, key, **kw: (
+                                    f"URL: {kw.get('url', '')}"
+                                    if "success" in key
+                                    else "Hint"
+                                )
+                                await calendar_link(update, MagicMock())
     message.reply_text.assert_called_once()
     call_args = message.reply_text.call_args[0][0]
     assert "abc43token" in call_args or "example.com" in call_args
@@ -216,9 +218,10 @@ async def test_calendar_link_denied_replies_access_denied():
             mock_user.id = 10
             mock_user.phone = None
             mock_get_user.return_value = mock_user
-            with patch("duty_teller.handlers.commands.config") as mock_cfg:
-                mock_cfg.can_access_miniapp.return_value = False
-                mock_cfg.can_access_miniapp_by_phone.return_value = False
+            with patch(
+                "duty_teller.handlers.commands.can_access_miniapp_for_telegram_user",
+                return_value=False,
+            ):
                 with patch("duty_teller.handlers.commands.get_lang", return_value="en"):
                     with patch("duty_teller.handlers.commands.t") as mock_t:
                         mock_t.return_value = "Access denied"

tests/test_handlers_init.py

@@ -9,5 +9,5 @@ def test_register_handlers_adds_all_handlers():
     """register_handlers: adds command, import, group pin handlers and error handler."""
     mock_app = MagicMock()
     register_handlers(mock_app)
-    assert mock_app.add_handler.call_count >= 9
+    assert mock_app.add_handler.call_count >= 10
     assert mock_app.add_error_handler.call_count == 1

tests/test_repository_roles.py

@@ -0,0 +1,172 @@
+"""Tests for role-related repository: get_user_role, set_user_role, is_admin, can_access_miniapp."""
+
+from unittest.mock import patch
+
+import pytest
+from sqlalchemy import create_engine
+from sqlalchemy.orm import sessionmaker
+
+from duty_teller.db.models import Base, Role
+from duty_teller.db.repository import (
+    ROLE_USER,
+    ROLE_ADMIN,
+    get_user_by_username,
+    get_user_role,
+    set_user_role,
+    is_admin_for_telegram_user,
+    can_access_miniapp_for_telegram_user,
+    can_access_miniapp_for_user,
+    get_or_create_user,
+)
+
+
+@pytest.fixture
+def session():
+    """Session with roles and users tables; roles 'user' and 'admin' inserted."""
+    engine = create_engine(
+        "sqlite:///:memory:", connect_args={"check_same_thread": False}
+    )
+    Base.metadata.create_all(engine)
+    Session = sessionmaker(bind=engine, autocommit=False, autoflush=False)
+    s = Session()
+    try:
+        for name in (ROLE_USER, ROLE_ADMIN):
+            r = Role(name=name)
+            s.add(r)
+        s.commit()
+        yield s
+    finally:
+        s.close()
+        engine.dispose()
+
+
+@pytest.fixture
+def user_no_role(session):
+    """User with telegram_user_id set, no role."""
+    u = get_or_create_user(
+        session,
+        telegram_user_id=100,
+        full_name="No Role",
+        username="norole",
+    )
+    return u
+
+
+@pytest.fixture
+def role_user(session):
+    return session.query(Role).filter(Role.name == ROLE_USER).first()
+
+
+@pytest.fixture
+def role_admin(session):
+    return session.query(Role).filter(Role.name == ROLE_ADMIN).first()
+
+
+def test_get_user_role_none_when_no_role(session, user_no_role):
+    assert get_user_role(session, user_no_role.id) is None
+
+
+def test_get_user_role_returns_name(session, user_no_role, role_user, role_admin):
+    user_no_role.role_id = role_user.id
+    session.commit()
+    assert get_user_role(session, user_no_role.id) == ROLE_USER
+    user_no_role.role_id = role_admin.id
+    session.commit()
+    assert get_user_role(session, user_no_role.id) == ROLE_ADMIN
+
+
+def test_set_user_role(session, user_no_role, role_user):
+    updated = set_user_role(session, user_no_role.id, ROLE_USER)
+    assert updated is not None
+    assert updated.id == user_no_role.id
+    assert updated.role_id == role_user.id
+    session.refresh(user_no_role)
+    assert user_no_role.role.name == ROLE_USER
+
+
+def test_set_user_role_invalid_name_returns_none(session, user_no_role):
+    assert set_user_role(session, user_no_role.id, "superuser") is None
+    assert set_user_role(session, 99999, ROLE_USER) is None
+
+
+@patch("duty_teller.db.repository.config.is_admin")
+@patch("duty_teller.db.repository.config.is_admin_by_phone")
+def test_is_admin_for_telegram_user_from_db_role(
+    mock_admin_phone, mock_admin, session, user_no_role, role_admin, role_user
+):
+    mock_admin.return_value = False
+    mock_admin_phone.return_value = False
+    user_no_role.role_id = role_user.id
+    session.commit()
+    assert is_admin_for_telegram_user(session, 100) is False
+    user_no_role.role_id = role_admin.id
+    session.commit()
+    assert is_admin_for_telegram_user(session, 100) is True
+    mock_admin.assert_not_called()
+    mock_admin_phone.assert_not_called()
+
+
+@patch("duty_teller.db.repository.config.is_admin")
+@patch("duty_teller.db.repository.config.is_admin_by_phone")
+def test_is_admin_for_telegram_user_fallback_when_no_role(
+    mock_admin_phone, mock_admin, session, user_no_role
+):
+    mock_admin.return_value = True
+    mock_admin_phone.return_value = False
+    assert is_admin_for_telegram_user(session, 100) is True
+    mock_admin.assert_called_once()
+    mock_admin.return_value = False
+    mock_admin_phone.return_value = True
+    assert is_admin_for_telegram_user(session, 100) is True
+
+
+@patch("duty_teller.db.repository.config.is_admin")
+@patch("duty_teller.db.repository.config.is_admin_by_phone")
+def test_is_admin_for_telegram_user_false_when_no_user(
+    mock_admin_phone, mock_admin, session
+):
+    mock_admin.return_value = False
+    mock_admin_phone.return_value = False
+    assert is_admin_for_telegram_user(session, 99999) is False
+
+
+def test_can_access_miniapp_for_telegram_user_no_user(session):
+    assert can_access_miniapp_for_telegram_user(session, 99999) is False
+
+
+def test_can_access_miniapp_for_telegram_user_with_role(
+    session, user_no_role, role_user, role_admin
+):
+    user_no_role.role_id = role_user.id
+    session.commit()
+    assert can_access_miniapp_for_telegram_user(session, 100) is True
+    user_no_role.role_id = role_admin.id
+    session.commit()
+    assert can_access_miniapp_for_telegram_user(session, 100) is True
+
+
+@patch("duty_teller.db.repository.config.is_admin")
+@patch("duty_teller.db.repository.config.is_admin_by_phone")
+def test_can_access_miniapp_for_telegram_user_fallback_when_no_role(
+    mock_admin_phone, mock_admin, session, user_no_role
+):
+    mock_admin.return_value = False
+    mock_admin_phone.return_value = False
+    assert can_access_miniapp_for_telegram_user(session, 100) is False
+    mock_admin.return_value = True
+    assert can_access_miniapp_for_telegram_user(session, 100) is True
+
+
+def test_can_access_miniapp_for_user_none(session):
+    assert can_access_miniapp_for_user(session, None) is False
+
+
+def test_get_user_by_username(session, user_no_role):
+    u = get_user_by_username(session, "norole")
+    assert u is not None and u.id == user_no_role.id
+    u2 = get_user_by_username(session, "@norole")
+    assert u2 is not None and u2.id == user_no_role.id
+    u3 = get_user_by_username(session, "NOROLE")
+    assert u3 is not None and u3.id == user_no_role.id
+    assert get_user_by_username(session, "unknown") is None
+    assert get_user_by_username(session, "") is None