Merge pull request #217 from theunkn0wn1/fix/SPARK-243

[SPARK-243] Fix vhost processing for all users

Author: theunkn0wn1

src/packages/user/user.py

@@ -12,13 +12,14 @@
 """
 from __future__ import annotations  # enable forward-references
 
-from dataclasses import dataclass
+import attr
+import cattr
 from typing import Union, Optional
 
 from pydle import BasicClient
 
 
-@dataclass(frozen=True)
+@attr.define(frozen=True)
 class User:
     """
     Represents an IRC user
@@ -50,7 +51,15 @@ async def from_pydle(cls, bot: BasicClient, nickname: str) -> Optional[User]:
 
         # if we got a object back
         if data:
-            return cls(**data)
+            result = cls(**data)
+
+            # This needs to be an evolve as the cls is frozen.
+            # That and wanting to avoid a converter on the class itself.
+
+            # Inspection false-positive. suppress.
+            # noinspection PyDataclass
+            result = attr.evolve(inst=result, hostname=cls.process_vhost(result.hostname))
+            return result
 
     @classmethod
     def process_vhost(cls, vhost: Union[str, None]) -> Optional[str]:
@@ -77,5 +86,10 @@ def process_vhost(cls, vhost: Union[str, None]) -> Optional[str]:
         # identify the role
         host = vhost.rsplit(".", 3)[-3]
 
+        # special case: privileged user naming scheme
+        if '@' in host:
+            # Split off the user from the host and return the group
+            host = host.split('@', maxsplit=1)[-1]
+
         # return the corresponding vhost
         return f"{host}.fuelrats.com"

tests/fixtures/mock_bot.py

@@ -33,7 +33,7 @@ def __init__(self, *args, **kwargs):
             "some_recruit": {
                 "nickname": "some_recruit",
                 "username": "ident_ed_your_car_sorry_bad_pun",
-                "hostname": "recruit.fuelrats.com",
+                "hostname": "some-recruit@some-recruit.recruit.fuelrats.com",
                 "away": False,
                 "away_message": None,
                 "account": None,
@@ -44,18 +44,28 @@ def __init__(self, *args, **kwargs):
             "some_ov": {
                 "nickname": "some_ov",
                 "username": "ill_stop",
-                "hostname": "overseer.fuelrats.com",
+                "hostname": "some_ov@some-ov.overseer.fuelrats.com",
                 "away": False,
                 "away_message": None,
                 "account": None,
                 "identified": True,
                 "realname": "Stop sign",
 
             },
+            "some_rat": {
+                "nickname": "some_rat",
+                "username": "ratlingDelux",
+                "hostname": "delux@delux.rat.fuelrats.com",
+                "away": False,
+                "away_message": None,
+                "account": None,
+                "identified": True,
+                "realname": "snakeeyes",
+            },
             "some_admin": {
                 "nickname": "some_admin",
                 "username": "SirRaymondLuxuryYacht",
-                "hostname": "admin.fuelrats.com",
+                "hostname": "reality@netadmin.fuelrats.com",
                 "away": False,
                 "away_message": None,
                 "account": None,

tests/unit/test_user.py

@@ -10,28 +10,28 @@
 """
 import pytest
 
+from src.packages.permissions import has_required_permission, ADMIN
 from src.packages.user.user import User
 
 pytestmark = [pytest.mark.unit, pytest.mark.user]
 
 
-@pytest.mark.parametrize("expected_host", [
-    "recruit.fuelrats.com",
-    "rat.fuelrats.com",
-    "dispatch.fuelrats.com",
-    "overseer.fuelrats.com",
-    "op.fuelrats.com",
-    "techrat.fuelrats.com",
-    "netadmin.fuelrats.com",
-    "admin.fuelrats.com",
+@pytest.mark.parametrize("expected_host, payload", [
+    ("recruit.fuelrats.com", "someone@someone.recruit.fuelrats.com"),
+    ("rat.fuelrats.com", "someone@someone.rat.fuelrats.com"),
+    ("dispatch.fuelrats.com", "someone@someone.dispatch.fuelrats.com"),
+    ("overseer.fuelrats.com", "someone@someone.overseer.fuelrats.com"),
+    ("op.fuelrats.com", "someone@someone.op.fuelrats.com"),
+    ("techrat.fuelrats.com", "someone@someone.techrat.fuelrats.com"),
+    ("netadmin.fuelrats.com", "someone@someone.netadmin.fuelrats.com"),
+    ("admin.fuelrats.com", "someone@someone.admin.fuelrats.com"),
+    ("netadmin.fuelrats.com", "reality@netadmin.fuelrats.com")
 ])
-@pytest.mark.parametrize("prefix", ["potato.", "Orbital.", ""])
-def test_process_vhost(prefix: str, expected_host: str):
+def test_process_vhost(payload: str, expected_host: str):
     """
     Asserts vhost processing functions as expected
     """
-    mixed_host = f"{prefix}{expected_host}"
-    assert User.process_vhost(mixed_host) == expected_host
+    assert User.process_vhost(payload) == expected_host
 
 
 def test_process_vhost_orange():
@@ -110,7 +110,7 @@ async def test_user_from_whois_existing_user(bot_fx):
     assert data['identified'] == my_user.identified
     assert data['account'] == my_user.account
     assert "some_recruit" == my_user.nickname
-    assert data['hostname'] == my_user.hostname
+    assert data['hostname'].endswith(my_user.hostname)
     assert data['username'] == my_user.username
     assert data['realname'] == my_user.realname
 
@@ -162,6 +162,10 @@ async def test_user_eq(data: dict, monkeypatch, bot_fx):
     assert user_alpha == user_beta
 
 
+async def test_effective_permission(bot_fx):
+    user = await User.from_pydle(bot_fx, "some_admin")
+    has_required_permission(user, ADMIN)
+
 @pytest.mark.regressions
 def test_hash(user_fx):
     assert hash(user_fx)