From 5ebbfab0ff738ccf35ede644d598f1551ed418ec Mon Sep 17 00:00:00 2001
From: Marek Wolan <marek.wolan@methods.co.uk>
Date: Tue, 1 Aug 2023 10:02:13 +0100
Subject: [PATCH 01/16] Create some files for domain sim

---
 src/primaite/simulator/domain_controller/__init__.py | 0
 src/primaite/simulator/domain_controller/account.py  | 8 ++++++++
 2 files changed, 8 insertions(+)
 create mode 100644 src/primaite/simulator/domain_controller/__init__.py
 create mode 100644 src/primaite/simulator/domain_controller/account.py

diff --git a/src/primaite/simulator/domain_controller/__init__.py b/src/primaite/simulator/domain_controller/__init__.py
new file mode 100644
index 00000000..e69de29b
diff --git a/src/primaite/simulator/domain_controller/account.py b/src/primaite/simulator/domain_controller/account.py
new file mode 100644
index 00000000..1f3ac900
--- /dev/null
+++ b/src/primaite/simulator/domain_controller/account.py
@@ -0,0 +1,8 @@
+"""User account simulation."""
+from primaite.simulator.core import SimComponent
+
+
+class Account(SimComponent):
+    """User accounts."""
+
+    uid: int

From 091b4a801dc5a7f558fdea273fcbed579f950021 Mon Sep 17 00:00:00 2001
From: Marek Wolan <marek.wolan@methods.co.uk>
Date: Wed, 2 Aug 2023 13:43:31 +0100
Subject: [PATCH 02/16] Make some progress on accounts

---
 src/primaite/simulator/domain/__init__.py     |  3 +
 src/primaite/simulator/domain/account.py      | 92 +++++++++++++++++++
 src/primaite/simulator/domain/controller.py   | 13 +++
 .../simulator/domain_controller/__init__.py   |  0
 .../simulator/domain_controller/account.py    |  8 --
 5 files changed, 108 insertions(+), 8 deletions(-)
 create mode 100644 src/primaite/simulator/domain/__init__.py
 create mode 100644 src/primaite/simulator/domain/account.py
 create mode 100644 src/primaite/simulator/domain/controller.py
 delete mode 100644 src/primaite/simulator/domain_controller/__init__.py
 delete mode 100644 src/primaite/simulator/domain_controller/account.py

diff --git a/src/primaite/simulator/domain/__init__.py b/src/primaite/simulator/domain/__init__.py
new file mode 100644
index 00000000..6f59cf49
--- /dev/null
+++ b/src/primaite/simulator/domain/__init__.py
@@ -0,0 +1,3 @@
+from primaite.simulator.domain.account import Account
+
+__all__ = ["Account"]
diff --git a/src/primaite/simulator/domain/account.py b/src/primaite/simulator/domain/account.py
new file mode 100644
index 00000000..374675a0
--- /dev/null
+++ b/src/primaite/simulator/domain/account.py
@@ -0,0 +1,92 @@
+"""User account simulation."""
+from enum import Enum
+from typing import Dict, List, Set, TypeAlias
+
+from primaite import getLogger
+from primaite.simulator.core import SimComponent
+
+_LOGGER = getLogger(__name__)
+
+
+__temp_node = TypeAlias()  # placeholder while nodes don't exist
+
+
+class AccountType(Enum):
+    """Whether the account is intended for a user to log in or for a service to use."""
+
+    service = 1
+    "Service accounts are used to grant permissions to software on nodes to perform actions"
+    user = 2
+    "User accounts are used to allow agents to log in and perform actions"
+
+
+class AccountGroup(Enum):
+    """Permissions are set at group-level and accounts can belong to these groups."""
+
+    local_user = 1
+    "For performing basic actions on a node"
+    domain_user = 2
+    "For performing basic actions to the domain"
+    local_admin = 3
+    "For full access to actions on a node"
+    domain_admin = 4
+    "For full access"
+
+
+class AccountStatus(Enum):
+    """Whether the account is active."""
+
+    enabled = 1
+    disabled = 2
+
+
+class Account(SimComponent):
+    """User accounts."""
+
+    num_logons: int = 0
+    "The number of times this account was logged into since last reset."
+    num_logoffs: int = 0
+    "The number of times this account was logged out of since last reset."
+    num_group_changes: int = 0
+    "The number of times this account was moved in or out of an AccountGroup."
+    username: str
+    "Account username."
+    password: str
+    "Account password."
+    account_type: AccountType
+    "Account Type, currently this can be service account (used by apps) or user account."
+    domain_groups: Set[AccountGroup] = []
+    "Domain-wide groups that this account belongs to."
+    local_groups: Dict[__temp_node, List[AccountGroup]]
+    "For each node, whether this account has local/admin privileges on that node."
+    status: AccountStatus = AccountStatus.disabled
+
+    def add_to_domain_group(self, group: AccountGroup) -> None:
+        """
+        Add this account to a domain group.
+
+        If the account is already a member of this group, nothing happens.
+
+        :param group: The group to which to add this account.
+        :type group: AccountGroup
+        """
+        self.domain_groups.add(group)
+
+    def remove_from_domain_group(self, group: AccountGroup) -> None:
+        """
+        Remove this account from a domain group.
+
+        If the account is already not a member of that group, nothing happens.
+
+        :param group: The group from which this account should be removed.
+        :type group: AccountGroup
+        """
+        self.domain_groups.discard(group)
+
+    def enable_account(self):
+        """Set the status to enabled."""
+        self.status = AccountStatus.enabled
+
+    def disable_account(self):
+        """Set the status to disabled."""
+        self.status = AccountStatus.disabled
diff --git a/src/primaite/simulator/domain/controller.py b/src/primaite/simulator/domain/controller.py
new file mode 100644
index 00000000..5a14e80e
--- /dev/null
+++ b/src/primaite/simulator/domain/controller.py
@@ -0,0 +1,13 @@
+from typing import Set, TypeAlias
+
+from primaite.simulator.core import SimComponent
+from primaite.simulator.domain import Account
+
+__temp_node = TypeAlias()  # placeholder while nodes don't exist
+
+
+class DomainController(SimComponent):
+    """Main object for controlling the domain."""
+
+    nodes: Set(__temp_node) = set()
+    accounts: Set(Account) = set()
diff --git a/src/primaite/simulator/domain_controller/__init__.py b/src/primaite/simulator/domain_controller/__init__.py
deleted file mode 100644
index e69de29b..00000000
diff --git a/src/primaite/simulator/domain_controller/account.py b/src/primaite/simulator/domain_controller/account.py
deleted file mode 100644
index 1f3ac900..00000000
--- a/src/primaite/simulator/domain_controller/account.py
+++ /dev/null
@@ -1,8 +0,0 @@
-"""User account simulation."""
-from primaite.simulator.core import SimComponent
-
-
-class Account(SimComponent):
-    """User accounts."""
-
-    uid: int

From 3a2840bed850de3fb2c57c55fc23bdee4fc55285 Mon Sep 17 00:00:00 2001
From: Marek Wolan <marek.wolan@methods.co.uk>
Date: Thu, 3 Aug 2023 13:09:04 +0100
Subject: [PATCH 03/16] Overhaul sim component for permission management.

---
 src/primaite/simulator/core.py              | 131 +++++++++++++++++---
 src/primaite/simulator/domain/__init__.py   |   4 +-
 src/primaite/simulator/domain/account.py    |  46 +++----
 src/primaite/simulator/domain/controller.py |  51 +++++++-
 4 files changed, 182 insertions(+), 50 deletions(-)

diff --git a/src/primaite/simulator/core.py b/src/primaite/simulator/core.py
index c3130116..eaedc85a 100644
--- a/src/primaite/simulator/core.py
+++ b/src/primaite/simulator/core.py
@@ -1,13 +1,123 @@
 """Core of the PrimAITE Simulator."""
-from abc import abstractmethod
-from typing import Callable, Dict, List
+from abc import ABC, abstractmethod
+from typing import Callable, Dict, List, Optional
 
-from pydantic import BaseModel
+from pydantic import BaseModel, ConfigDict
+
+from primaite import getLogger
+from primaite.simulator.domain import AccountGroup
+
+_LOGGER = getLogger(__name__)
+
+
+class ActionPermissionValidator(ABC):
+    """
+    Base class for action validators.
+
+    The permissions manager is designed to be generic. So, although in the first instance the permissions
+    are evaluated purely on membership to AccountGroup, this class can support validating permissions based on any
+    arbitrary criteria.
+    """
+
+    @abstractmethod
+    def __call__(self, request: List[str], context: Dict) -> bool:
+        """TODO."""
+        pass
+
+
+class AllowAllValidator(ActionPermissionValidator):
+    """Always allows the action."""
+
+    def __call__(self, request: List[str], context: Dict) -> bool:
+        """Always allow the action."""
+        return True
+
+
+class GroupMembershipValidator(ActionPermissionValidator):
+    """Permit actions based on group membership."""
+
+    def __init__(self, allowed_groups: List[AccountGroup]) -> None:
+        """TODO."""
+        self.allowed_groups = allowed_groups
+
+    def __call__(self, request: List[str], context: Dict) -> bool:
+        """Permit the action if the request comes from an account which belongs to the right group."""
+        # if context request source is part of any groups mentioned in self.allow_groups, return true, otherwise false
+        requestor_groups: List[str] = context["request_source"]["groups"]
+        for allowed_group in self.allowed_groups:
+            if allowed_group.name in requestor_groups:
+                return True
+        return False
+
+
+class Action:
+    """
+    This object stores data related to a single action.
+
+    This includes the callable that can execute the action request, and the validator that will decide whether
+    the action can be performed or not.
+    """
+
+    def __init__(self, func: Callable[[List[str], Dict], None], validator: ActionPermissionValidator) -> None:
+        """
+        Save the functions that are for this action.
+
+        Here's a description for the intended use of both of these.
+
+        ``func`` is a function that accepts a request and a context dict. Typically this would be a lambda function
+        that invokes a class method of your SimComponent. For example if the component is a node and the action is for
+        turning it off, then the SimComponent should have a turn_off(self) method that does not need to accept any args.
+        Then, this Action will be given something like ``func = lambda request, context: self.turn_off()``.
+
+        :param func: Function that performs the request.
+        :type func: Callable[[List[str], Dict], None]
+        :param validator: Function that checks if the request is authenticated given the context.
+        :type validator: ActionPermissionValidator
+        """
+        self.func: Callable[[List[str], Dict], None] = func
+        self.validator: ActionPermissionValidator = validator
+
+
+class ActionManager:
+    """TODO."""
+
+    def __init__(self) -> None:
+        """TODO."""
+        self.actions: Dict[str, Action]
+
+    def process_request(self, request: List[str], context: Dict) -> None:
+        """Process action request."""
+        action_key = request[0]
+
+        if action_key not in self.actions:
+            msg = (
+                f"Action request {request} could not be processed because {action_key} is not a valid action",
+                "within this ActionManager",
+            )
+            _LOGGER.error(msg)
+            raise RuntimeError(msg)
+
+        action = self.actions[action_key]
+        action_options = request[1:]
+
+        if not action.validator(action_options, context):
+            _LOGGER.debug(f"Action request {request} was denied due to insufficient permissions")
+            return
+
+        action.func(action_options, context)
 
 
 class SimComponent(BaseModel):
     """Extension of pydantic BaseModel with additional methods that must be defined by all classes in  the simulator."""
 
+    model_config = ConfigDict(arbitrary_types_allowed=True)
+    uuid: str
+    "The component UUID."
+
+    def __init__(self, **kwargs) -> None:
+        self.action_manager: Optional[ActionManager] = None
+        super().__init__(**kwargs)
+
     @abstractmethod
     def describe_state(self) -> Dict:
         """
@@ -19,7 +129,7 @@ class SimComponent(BaseModel):
         """
         return {}
 
-    def apply_action(self, action: List[str]) -> None:
+    def apply_action(self, action: List[str], context: Dict = {}) -> None:
         """
         Apply an action to a simulation component. Action data is passed in as a 'namespaced' list of strings.
 
@@ -34,16 +144,9 @@ class SimComponent(BaseModel):
         :param action: List describing the action to apply to this object.
         :type action: List[str]
         """
-        possible_actions = self._possible_actions()
-        if action[0] in possible_actions:
-            # take the first element off the action list and pass the remaining arguments to the corresponding action
-            # function
-            possible_actions[action.pop(0)](action)
-        else:
-            raise ValueError(f"{self.__class__.__name__} received invalid action {action}")
-
-    def _possible_actions(self) -> Dict[str, Callable[[List[str]], None]]:
-        return {}
+        if self.action_manager is None:
+            return
+        self.action_manager.process_request(action, context)
 
     def apply_timestep(self, timestep: int) -> None:
         """
diff --git a/src/primaite/simulator/domain/__init__.py b/src/primaite/simulator/domain/__init__.py
index 6f59cf49..0e23133f 100644
--- a/src/primaite/simulator/domain/__init__.py
+++ b/src/primaite/simulator/domain/__init__.py
@@ -1,3 +1,3 @@
-from primaite.simulator.domain.account import Account
+from primaite.simulator.domain.account import Account, AccountGroup, AccountType
 
-__all__ = ["Account"]
+__all__ = ["Account", "AccountGroup", "AccountType"]
diff --git a/src/primaite/simulator/domain/account.py b/src/primaite/simulator/domain/account.py
index 374675a0..c134e916 100644
--- a/src/primaite/simulator/domain/account.py
+++ b/src/primaite/simulator/domain/account.py
@@ -1,6 +1,6 @@
 """User account simulation."""
 from enum import Enum
-from typing import Dict, List, Set, TypeAlias
+from typing import Callable, Dict, List, TypeAlias
 
 from primaite import getLogger
 from primaite.simulator.core import SimComponent
@@ -40,6 +40,14 @@ class AccountStatus(Enum):
     disabled = 2
 
 
+class PasswordPolicyLevel(Enum):
+    """Complexity requirements for account passwords."""
+
+    low = 1
+    medium = 2
+    high = 3
+
+
 class Account(SimComponent):
     """User accounts."""
 
@@ -55,38 +63,18 @@ class Account(SimComponent):
     "Account password."
     account_type: AccountType
     "Account Type, currently this can be service account (used by apps) or user account."
-    domain_groups: Set[AccountGroup] = []
-    "Domain-wide groups that this account belongs to."
-    local_groups: Dict[__temp_node, List[AccountGroup]]
-    "For each node, whether this account has local/admin privileges on that node."
     status: AccountStatus = AccountStatus.disabled
 
-    def add_to_domain_group(self, group: AccountGroup) -> None:
-        """
-        Add this account to a domain group.
-
-        If the account is already a member of this group, nothing happens.
-
-        :param group: The group to which to add this account.
-        :type group: AccountGroup
-        """
-        self.domain_groups.add(group)
-
-    def remove_from_domain_group(self, group: AccountGroup) -> None:
-        """
-        Remove this account from a domain group.
-
-        If the account is already not a member of that group, nothing happens.
-
-        :param group: The group from which this account should be removed.
-        :type group: AccountGroup
-        """
-        self.domain_groups.discard(group)
-
-    def enable_account(self):
+    def enable(self):
         """Set the status to enabled."""
         self.status = AccountStatus.enabled
 
-    def disable_account(self):
+    def disable(self):
         """Set the status to disabled."""
         self.status = AccountStatus.disabled
+
+    def _possible_actions(self) -> Dict[str, Callable[[List[str]], None]]:
+        return {
+            "enable": self.enable,
+            "disable": self.disable,
+        }
diff --git a/src/primaite/simulator/domain/controller.py b/src/primaite/simulator/domain/controller.py
index 5a14e80e..c9165bbf 100644
--- a/src/primaite/simulator/domain/controller.py
+++ b/src/primaite/simulator/domain/controller.py
@@ -1,13 +1,54 @@
-from typing import Set, TypeAlias
+from typing import Dict, Final, List, TypeAlias
 
 from primaite.simulator.core import SimComponent
-from primaite.simulator.domain import Account
+from primaite.simulator.domain import Account, AccountGroup, AccountType
 
-__temp_node = TypeAlias()  # placeholder while nodes don't exist
+# placeholder while these objects don't yet exist
+__temp_node = TypeAlias()
+__temp_application = TypeAlias()
+__temp_folder = TypeAlias()
+__temp_file = TypeAlias()
 
 
 class DomainController(SimComponent):
     """Main object for controlling the domain."""
 
-    nodes: Set(__temp_node) = set()
-    accounts: Set(Account) = set()
+    # owned objects
+    accounts: List(Account) = []
+    groups: Final[List[AccountGroup]] = list(AccountGroup)
+
+    group_membership: Dict[AccountGroup, List[Account]]
+
+    # references to non-owned objects
+    nodes: List(__temp_node) = []
+    applications: List(__temp_application) = []
+    folders: List(__temp_folder) = []
+    files: List(__temp_file) = []
+
+    def register_account(self, account: Account) -> None:
+        """TODO."""
+        ...
+
+    def deregister_account(self, account: Account) -> None:
+        """TODO."""
+        ...
+
+    def create_account(self, username: str, password: str, account_type: AccountType) -> Account:
+        """TODO."""
+        ...
+
+    def rotate_all_credentials(self) -> None:
+        """TODO."""
+        ...
+
+    def rotate_account_credentials(self, account: Account) -> None:
+        """TODO."""
+        ...
+
+    def add_account_to_group(self, account: Account, group: AccountGroup) -> None:
+        """TODO."""
+        ...
+
+    def remove_account_from_group(self, account: Account, group: AccountGroup) -> None:
+        """TODO."""
+        ...

From 94617c57a4a70b96840004862d1cfc7467283cf2 Mon Sep 17 00:00:00 2001
From: Marek Wolan <marek.wolan@methods.co.uk>
Date: Thu, 3 Aug 2023 13:24:27 +0100
Subject: [PATCH 04/16] Make register and deregister acct private

---
 src/primaite/simulator/domain/controller.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/primaite/simulator/domain/controller.py b/src/primaite/simulator/domain/controller.py
index c9165bbf..bdb5fbb0 100644
--- a/src/primaite/simulator/domain/controller.py
+++ b/src/primaite/simulator/domain/controller.py
@@ -25,11 +25,11 @@ class DomainController(SimComponent):
     folders: List(__temp_folder) = []
     files: List(__temp_file) = []
 
-    def register_account(self, account: Account) -> None:
+    def _register_account(self, account: Account) -> None:
         """TODO."""
         ...
 
-    def deregister_account(self, account: Account) -> None:
+    def _deregister_account(self, account: Account) -> None:
         """TODO."""
         ...
 

From 2a680c1e4817764f92887a7e15d0807b5c4f5d31 Mon Sep 17 00:00:00 2001
From: Marek Wolan <marek.wolan@methods.co.uk>
Date: Thu, 3 Aug 2023 16:26:33 +0100
Subject: [PATCH 05/16] Test my validators

---
 src/primaite/simulator/core.py                |  32 +---
 src/primaite/simulator/domain/__init__.py     |   3 -
 src/primaite/simulator/domain/account.py      |  18 +-
 src/primaite/simulator/domain/controller.py   |  66 +++++--
 .../component_creation/__init__.py            |   0
 .../test_permission_system.py                 | 171 ++++++++++++++++++
 6 files changed, 235 insertions(+), 55 deletions(-)
 create mode 100644 tests/integration_tests/component_creation/__init__.py
 create mode 100644 tests/integration_tests/component_creation/test_permission_system.py

diff --git a/src/primaite/simulator/core.py b/src/primaite/simulator/core.py
index eaedc85a..17e09f85 100644
--- a/src/primaite/simulator/core.py
+++ b/src/primaite/simulator/core.py
@@ -1,11 +1,11 @@
 """Core of the PrimAITE Simulator."""
 from abc import ABC, abstractmethod
 from typing import Callable, Dict, List, Optional
+from uuid import uuid4
 
-from pydantic import BaseModel, ConfigDict
+from pydantic import BaseModel, ConfigDict, Extra
 
 from primaite import getLogger
-from primaite.simulator.domain import AccountGroup
 
 _LOGGER = getLogger(__name__)
 
@@ -33,23 +33,6 @@ class AllowAllValidator(ActionPermissionValidator):
         return True
 
 
-class GroupMembershipValidator(ActionPermissionValidator):
-    """Permit actions based on group membership."""
-
-    def __init__(self, allowed_groups: List[AccountGroup]) -> None:
-        """TODO."""
-        self.allowed_groups = allowed_groups
-
-    def __call__(self, request: List[str], context: Dict) -> bool:
-        """Permit the action if the request comes from an account which belongs to the right group."""
-        # if context request source is part of any groups mentioned in self.allow_groups, return true, otherwise false
-        requestor_groups: List[str] = context["request_source"]["groups"]
-        for allowed_group in self.allowed_groups:
-            if allowed_group.name in requestor_groups:
-                return True
-        return False
-
-
 class Action:
     """
     This object stores data related to a single action.
@@ -83,7 +66,7 @@ class ActionManager:
 
     def __init__(self) -> None:
         """TODO."""
-        self.actions: Dict[str, Action]
+        self.actions: Dict[str, Action] = {}
 
     def process_request(self, request: List[str], context: Dict) -> None:
         """Process action request."""
@@ -106,17 +89,20 @@ class ActionManager:
 
         action.func(action_options, context)
 
+    def add_action(self, name: str, action: Action) -> None:
+        self.actions[name] = action
+
 
 class SimComponent(BaseModel):
     """Extension of pydantic BaseModel with additional methods that must be defined by all classes in  the simulator."""
 
-    model_config = ConfigDict(arbitrary_types_allowed=True)
-    uuid: str
+    model_config = ConfigDict(arbitrary_types_allowed=True, extra=Extra.allow)
+    uuid: str = str(uuid4())
     "The component UUID."
 
     def __init__(self, **kwargs) -> None:
-        self.action_manager: Optional[ActionManager] = None
         super().__init__(**kwargs)
+        self.action_manager: Optional[ActionManager] = None
 
     @abstractmethod
     def describe_state(self) -> Dict:
diff --git a/src/primaite/simulator/domain/__init__.py b/src/primaite/simulator/domain/__init__.py
index 0e23133f..e69de29b 100644
--- a/src/primaite/simulator/domain/__init__.py
+++ b/src/primaite/simulator/domain/__init__.py
@@ -1,3 +0,0 @@
-from primaite.simulator.domain.account import Account, AccountGroup, AccountType
-
-__all__ = ["Account", "AccountGroup", "AccountType"]
diff --git a/src/primaite/simulator/domain/account.py b/src/primaite/simulator/domain/account.py
index c134e916..0f59db2e 100644
--- a/src/primaite/simulator/domain/account.py
+++ b/src/primaite/simulator/domain/account.py
@@ -1,6 +1,6 @@
 """User account simulation."""
 from enum import Enum
-from typing import Callable, Dict, List, TypeAlias
+from typing import Any, Callable, Dict, List
 
 from primaite import getLogger
 from primaite.simulator.core import SimComponent
@@ -8,9 +8,6 @@ from primaite.simulator.core import SimComponent
 _LOGGER = getLogger(__name__)
 
 
-__temp_node = TypeAlias()  # placeholder while nodes don't exist
-
-
 class AccountType(Enum):
     """Whether the account is intended for a user to log in or for a service to use."""
 
@@ -20,19 +17,6 @@ class AccountType(Enum):
     "User accounts are used to allow agents to log in and perform actions"
 
 
-class AccountGroup(Enum):
-    """Permissions are set at group-level and accounts can belong to these groups."""
-
-    local_user = 1
-    "For performing basic actions on a node"
-    domain_user = 2
-    "For performing basic actions to the domain"
-    local_admin = 3
-    "For full access to actions on a node"
-    domain_admin = 4
-    "For full access"
-
-
 class AccountStatus(Enum):
     """Whether the account is active."""
 
diff --git a/src/primaite/simulator/domain/controller.py b/src/primaite/simulator/domain/controller.py
index bdb5fbb0..7cb3f4a6 100644
--- a/src/primaite/simulator/domain/controller.py
+++ b/src/primaite/simulator/domain/controller.py
@@ -1,29 +1,71 @@
-from typing import Dict, Final, List, TypeAlias
+from enum import Enum
+from typing import Any, Dict, Final, List
+
+from primaite.simulator.core import ActionPermissionValidator, SimComponent
+from primaite.simulator.domain.account import Account, AccountType
 
-from primaite.simulator.core import SimComponent
-from primaite.simulator.domain import Account, AccountGroup, AccountType
 
 # placeholder while these objects don't yet exist
-__temp_node = TypeAlias()
-__temp_application = TypeAlias()
-__temp_folder = TypeAlias()
-__temp_file = TypeAlias()
+class temp_node:
+    pass
+
+
+class temp_application:
+    pass
+
+
+class temp_folder:
+    pass
+
+
+class temp_file:
+    pass
+
+
+class AccountGroup(Enum):
+    """Permissions are set at group-level and accounts can belong to these groups."""
+
+    local_user = 1
+    "For performing basic actions on a node"
+    domain_user = 2
+    "For performing basic actions to the domain"
+    local_admin = 3
+    "For full access to actions on a node"
+    domain_admin = 4
+    "For full access"
+
+
+class GroupMembershipValidator(ActionPermissionValidator):
+    """Permit actions based on group membership."""
+
+    def __init__(self, allowed_groups: List[AccountGroup]) -> None:
+        """TODO."""
+        self.allowed_groups = allowed_groups
+
+    def __call__(self, request: List[str], context: Dict) -> bool:
+        """Permit the action if the request comes from an account which belongs to the right group."""
+        # if context request source is part of any groups mentioned in self.allow_groups, return true, otherwise false
+        requestor_groups: List[str] = context["request_source"]["groups"]
+        for allowed_group in self.allowed_groups:
+            if allowed_group.name in requestor_groups:
+                return True
+        return False
 
 
 class DomainController(SimComponent):
     """Main object for controlling the domain."""
 
     # owned objects
-    accounts: List(Account) = []
+    accounts: List[Account] = []
     groups: Final[List[AccountGroup]] = list(AccountGroup)
 
     group_membership: Dict[AccountGroup, List[Account]]
 
     # references to non-owned objects
-    nodes: List(__temp_node) = []
-    applications: List(__temp_application) = []
-    folders: List(__temp_folder) = []
-    files: List(__temp_file) = []
+    nodes: List[temp_node] = []
+    applications: List[temp_application] = []
+    folders: List[temp_folder] = []
+    files: List[temp_file] = []
 
     def _register_account(self, account: Account) -> None:
         """TODO."""
diff --git a/tests/integration_tests/component_creation/__init__.py b/tests/integration_tests/component_creation/__init__.py
new file mode 100644
index 00000000..e69de29b
diff --git a/tests/integration_tests/component_creation/test_permission_system.py b/tests/integration_tests/component_creation/test_permission_system.py
new file mode 100644
index 00000000..acc35b72
--- /dev/null
+++ b/tests/integration_tests/component_creation/test_permission_system.py
@@ -0,0 +1,171 @@
+from enum import Enum
+from typing import Dict, List, Literal
+
+import pytest
+
+from primaite.simulator.core import Action, ActionManager, AllowAllValidator, SimComponent
+from primaite.simulator.domain.controller import AccountGroup, GroupMembershipValidator
+
+
+def test_group_action_validation() -> None:
+    """Check that actions are denied when an unauthorised request is made."""
+
+    class Folder(SimComponent):
+        name: str
+
+        def describe_state(self) -> Dict:
+            return super().describe_state()
+
+    class Node(SimComponent):
+        name: str
+        folders: List[Folder] = []
+
+        def __init__(self, **kwargs):
+            super().__init__(**kwargs)
+            self.action_manager = ActionManager()
+
+            self.action_manager.add_action(
+                "create_folder",
+                Action(
+                    func=lambda request, context: self.create_folder(request[0]),
+                    validator=GroupMembershipValidator([AccountGroup.local_admin, AccountGroup.domain_admin]),
+                ),
+            )
+
+        def describe_state(self) -> Dict:
+            return super().describe_state()
+
+        def create_folder(self, folder_name: str) -> None:
+            new_folder = Folder(uuid="0000-0000-0001", name=folder_name)
+            self.folders.append(new_folder)
+
+        def remove_folder(self, folder: Folder) -> None:
+            self.folders = [x for x in self.folders if x is not folder]
+
+    permitted_context = {"request_source": {"agent": "BLUE", "account": "User1", "groups": ["local_admin"]}}
+
+    my_node = Node(uuid="0000-0000-1234", name="pc")
+    my_node.apply_action(["create_folder", "memes"], context=permitted_context)
+    assert len(my_node.folders) == 1
+    assert my_node.folders[0].name == "memes"
+
+    invalid_context = {"request_source": {"agent": "BLUE", "account": "User1", "groups": ["local_user", "domain_user"]}}
+
+    my_node.apply_action(["create_folder", "memes2"], context=invalid_context)
+    assert len(my_node.folders) == 1
+    assert my_node.folders[0].name == "memes"
+
+
+def test_hierarchical_action_with_validation() -> None:
+    """Check that validation works with sub-objects"""
+
+    class Application(SimComponent):
+        name: str
+        state: Literal["on", "off", "disabled"] = "off"
+
+        def __init__(self, **kwargs):
+            super().__init__(**kwargs)
+            self.action_manager = ActionManager()
+
+            self.action_manager.add_action(
+                "turn_on",
+                Action(
+                    func=lambda request, context: self.turn_on(),
+                    validator=AllowAllValidator(),
+                ),
+            )
+            self.action_manager.add_action(
+                "turn_off",
+                Action(
+                    func=lambda request, context: self.turn_off(),
+                    validator=AllowAllValidator(),
+                ),
+            )
+            self.action_manager.add_action(
+                "disable",
+                Action(
+                    func=lambda request, context: self.disable(),
+                    validator=GroupMembershipValidator([AccountGroup.local_admin, AccountGroup.domain_admin]),
+                ),
+            )
+            self.action_manager.add_action(
+                "enable",
+                Action(
+                    func=lambda request, context: self.enable(),
+                    validator=GroupMembershipValidator([AccountGroup.local_admin, AccountGroup.domain_admin]),
+                ),
+            )
+
+        def describe_state(self) -> Dict:
+            return super().describe_state()
+
+        def disable(self) -> None:
+            self.status = "disabled"
+
+        def enable(self) -> None:
+            if self.status == "disabled":
+                self.status = "off"
+
+        def turn_on(self) -> None:
+            if self.status == "off":
+                self.status = "on"
+
+        def turn_off(self) -> None:
+            if self.status == "on":
+                self.status = "off"
+
+    class Node(SimComponent):
+        name: str
+        state: Literal["on", "off"] = "on"
+        apps: List[Application] = []
+
+        def __init__(self, **kwargs):
+            super().__init__(**kwargs)
+            self.action_manager = ActionManager()
+
+            self.action_manager.add_action(
+                "apps",
+                Action(
+                    func=lambda request, context: self.send_action_to_app(request.pop(0), request, context),
+                    validator=AllowAllValidator(),
+                ),
+            )
+
+        def describe_state(self) -> Dict:
+            return super().describe_state()
+
+        def install_app(self, app_name: str) -> None:
+            new_app = Application(name=app_name)
+            self.apps.append(new_app)
+
+        def send_action_to_app(self, app_name: str, options: List[str], context: Dict):
+            for app in self.apps:
+                if app_name == app.name:
+                    app.apply_action(options)
+                    break
+            else:
+                msg = f"Node has no app with name {app_name}"
+                raise LookupError(msg)
+
+    my_node = Node(name="pc")
+    my_node.install_app("Chrome")
+    my_node.install_app("Firefox")
+
+    non_admin_context = {
+        "request_source": {"agent": "BLUE", "account": "User1", "groups": ["local_user", "domain_user"]}
+    }
+
+    admin_context = {
+        "request_source": {
+            "agent": "BLUE",
+            "account": "User1",
+            "groups": ["local_admin", "domain_admin", "local_user", "domain_user"],
+        }
+    }
+
+    my_node.apply_action(["apps", "Chrome", "disable"], non_admin_context)
+    my_node.apply_action(["apps", "Firefox", "turn_on"], non_admin_context)
+
+    assert my_node.apps[0].name == "Chrome"
+    assert my_node.apps[1].name == "Firefox"
+    assert my_node.apps[0].state == ...  # TODO: finish

From f0d7e03fd7548305c70638007f5bfc73829a9154 Mon Sep 17 00:00:00 2001
From: Marek Wolan <marek.wolan@methods.co.uk>
Date: Mon, 7 Aug 2023 10:55:29 +0100
Subject: [PATCH 06/16] Add docs and tests

---
 docs/source/simulation_structure.rst          | 52 +++++++++++++++++++
 src/primaite/simulator/domain/account.py      | 18 ++++---
 src/primaite/simulator/domain/controller.py   | 33 ++++++++++--
 .../test_permission_system.py                 | 51 ++++++++++++------
 .../_primaite/_simulator/_domain/__init__.py  |  0
 .../_simulator/_domain/test_account.py        | 18 +++++++
 .../_simulator/_domain/test_controller.py     |  0
 7 files changed, 147 insertions(+), 25 deletions(-)
 create mode 100644 tests/unit_tests/_primaite/_simulator/_domain/__init__.py
 create mode 100644 tests/unit_tests/_primaite/_simulator/_domain/test_account.py
 create mode 100644 tests/unit_tests/_primaite/_simulator/_domain/test_controller.py

diff --git a/docs/source/simulation_structure.rst b/docs/source/simulation_structure.rst
index 65373a72..479b3e7b 100644
--- a/docs/source/simulation_structure.rst
+++ b/docs/source/simulation_structure.rst
@@ -11,3 +11,55 @@ top level, there is an object called the ``SimulationController`` _(doesn't exis
 and a software controller for managing software and users.
 
 Each node of the simulation 'tree' has responsibility for creating, deleting, and updating its direct descendants.
+
+
+
+Actions
+=======
+Agents can interact with the simulation by using actions. Actions are standardised with the
+:py:class:`primaite.simulation.core.Action` class, which just holds a reference to two special functions.
+
+1. The action function itself, it must accept a `request` parameters which is a list of strings that describe what the
+   action should do. It must also accept a `context` dict which can house additional information surrounding the action.
+   For example, the context will typically include information about which entity intiated the action.
+2. A validator function. This function should return a boolean value that decides if the request is permitted or not.
+   It uses the same paramters as the action function.
+
+Action Permissions
+------------------
+When an agent tries to perform an action on a simulation component, that action will only be executed if the request is
+validated. For example, some actions can require that an agent is logged into an admin account. Each action defines its
+own permissions using an instance of :py:class:`primaite.simulation.core.ActionPermissionValidator`. The below code
+snippet demonstrates usage of the ``ActionPermissionValidator``.
+
+.. code:: python
+
+    from primaite.simulator.core import Action, ActionManager, SimComponent
+    from primaite.simulator.domain.controller import AccountGroup, GroupMembershipValidator
+
+    class Smartphone(SimComponent):
+        name: str
+        apps = []
+
+        def __init__(self, **kwargs):
+            super().__init__(**kwargs)
+            self.action_manager = ActionManager()
+
+            self.action_manager.add_action(
+                "reset_factory_settings",
+                Action(
+                    func = lambda request, context: self.reset_factory_settings(),
+                    validator = GroupMembershipValidator([AccountGroup.domain_admin]),
+                ),
+            )
+
+        def reset_factory_settings(self):
+            self.apps = []
+
+    phone = Smartphone(name="phone1")
+
+    # try to wipe the phone as a domain user, this will have no effect
+    phone.apply_action(["reset_factory_settings"], context={"request_source":{"groups":["domain_user"]})
+
+    # try to wipe the phone as an admin user, this will wipe the phone
+    phone.apply_action(["reset_factory_settings"], context={"request_source":{"groups":["domain_admin"]})
diff --git a/src/primaite/simulator/domain/account.py b/src/primaite/simulator/domain/account.py
index 0f59db2e..086022e6 100644
--- a/src/primaite/simulator/domain/account.py
+++ b/src/primaite/simulator/domain/account.py
@@ -1,6 +1,6 @@
 """User account simulation."""
 from enum import Enum
-from typing import Any, Callable, Dict, List
+from typing import Dict
 
 from primaite import getLogger
 from primaite.simulator.core import SimComponent
@@ -49,6 +49,10 @@ class Account(SimComponent):
     "Account Type, currently this can be service account (used by apps) or user account."
     status: AccountStatus = AccountStatus.disabled
 
+    def describe_state(self) -> Dict:
+        """Describe state for agent observations."""
+        return super().describe_state()
+
     def enable(self):
         """Set the status to enabled."""
         self.status = AccountStatus.enabled
@@ -57,8 +61,10 @@ class Account(SimComponent):
         """Set the status to disabled."""
         self.status = AccountStatus.disabled
 
-    def _possible_actions(self) -> Dict[str, Callable[[List[str]], None]]:
-        return {
-            "enable": self.enable,
-            "disable": self.disable,
-        }
+    def log_on(self) -> None:
+        """TODO."""
+        self.num_logons += 1
+
+    def log_off(self) -> None:
+        """TODO."""
+        self.num_logoffs += 1
diff --git a/src/primaite/simulator/domain/controller.py b/src/primaite/simulator/domain/controller.py
index 7cb3f4a6..e4a73b4e 100644
--- a/src/primaite/simulator/domain/controller.py
+++ b/src/primaite/simulator/domain/controller.py
@@ -1,5 +1,5 @@
 from enum import Enum
-from typing import Any, Dict, Final, List
+from typing import Dict, Final, List, Literal, Tuple
 
 from primaite.simulator.core import ActionPermissionValidator, SimComponent
 from primaite.simulator.domain.account import Account, AccountType
@@ -7,18 +7,26 @@ from primaite.simulator.domain.account import Account, AccountType
 
 # placeholder while these objects don't yet exist
 class temp_node:
+    """Placeholder for node class for type hinting purposes."""
+
     pass
 
 
 class temp_application:
+    """Placeholder for application class for type hinting purposes."""
+
     pass
 
 
 class temp_folder:
+    """Placeholder for folder class for type hinting purposes."""
+
     pass
 
 
 class temp_file:
+    """Placeholder for file class for type hinting purposes."""
+
     pass
 
 
@@ -59,9 +67,12 @@ class DomainController(SimComponent):
     accounts: List[Account] = []
     groups: Final[List[AccountGroup]] = list(AccountGroup)
 
-    group_membership: Dict[AccountGroup, List[Account]]
+    domain_group_membership: Dict[Literal[AccountGroup.domain_admin, AccountGroup.domain_user], List[Account]] = {}
+    local_group_membership: Dict[
+        Tuple(temp_node, Literal[AccountGroup.local_admin, AccountGroup.local_user]), List[Account]
+    ] = {}
 
-    # references to non-owned objects
+    # references to non-owned objects. Not sure if all are needed here.
     nodes: List[temp_node] = []
     applications: List[temp_application] = []
     folders: List[temp_folder] = []
@@ -79,6 +90,10 @@ class DomainController(SimComponent):
         """TODO."""
         ...
 
+    def delete_account(self, account: Account) -> None:
+        """TODO."""
+        ...
+
     def rotate_all_credentials(self) -> None:
         """TODO."""
         ...
@@ -94,3 +109,15 @@ class DomainController(SimComponent):
     def remove_account_from_group(self, account: Account, group: AccountGroup) -> None:
         """TODO."""
         ...
+
+    def check_account_permissions(self, account: Account, node: temp_node) -> List[AccountGroup]:
+        """Return a list of permission groups that this account has on this node."""
+        ...
+
+    def register_node(self, node: temp_node) -> None:
+        """TODO."""
+        ...
+
+    def deregister_node(self, node: temp_node) -> None:
+        """TODO."""
+        ...
diff --git a/tests/integration_tests/component_creation/test_permission_system.py b/tests/integration_tests/component_creation/test_permission_system.py
index acc35b72..93d0267c 100644
--- a/tests/integration_tests/component_creation/test_permission_system.py
+++ b/tests/integration_tests/component_creation/test_permission_system.py
@@ -8,7 +8,13 @@ from primaite.simulator.domain.controller import AccountGroup, GroupMembershipVa
 
 
 def test_group_action_validation() -> None:
-    """Check that actions are denied when an unauthorised request is made."""
+    """
+    Check that actions are denied when an unauthorised request is made.
+
+    This test checks the integration between SimComponent and the permissions validation system. First, we create a
+    basic node and folder class. We configure the node so that only admins can create a folder. Then, we try to create
+    a folder as both an admin user and a non-admin user.
+    """
 
     class Folder(SimComponent):
         name: str
@@ -42,22 +48,28 @@ def test_group_action_validation() -> None:
         def remove_folder(self, folder: Folder) -> None:
             self.folders = [x for x in self.folders if x is not folder]
 
+    # check that the folder is created when a local admin tried to do it
     permitted_context = {"request_source": {"agent": "BLUE", "account": "User1", "groups": ["local_admin"]}}
-
     my_node = Node(uuid="0000-0000-1234", name="pc")
     my_node.apply_action(["create_folder", "memes"], context=permitted_context)
     assert len(my_node.folders) == 1
     assert my_node.folders[0].name == "memes"
 
+    # check that the number of folders is still 1 even after attempting to create a second one without permissions
     invalid_context = {"request_source": {"agent": "BLUE", "account": "User1", "groups": ["local_user", "domain_user"]}}
-
     my_node.apply_action(["create_folder", "memes2"], context=invalid_context)
     assert len(my_node.folders) == 1
     assert my_node.folders[0].name == "memes"
 
 
 def test_hierarchical_action_with_validation() -> None:
-    """Check that validation works with sub-objects"""
+    """
+    Check that validation works with sub-objects.
+
+    This test creates a parent object (Node) and a child object (Application) which both accept actions. The node allows
+    action passthrough to applications. The purpose of this test is to check that after an action is passed through to
+    a child object, that the permission system still works as intended.
+    """
 
     class Application(SimComponent):
         name: str
@@ -100,19 +112,19 @@ def test_hierarchical_action_with_validation() -> None:
             return super().describe_state()
 
         def disable(self) -> None:
-            self.status = "disabled"
+            self.state = "disabled"
 
         def enable(self) -> None:
-            if self.status == "disabled":
-                self.status = "off"
+            if self.state == "disabled":
+                self.state = "off"
 
         def turn_on(self) -> None:
-            if self.status == "off":
-                self.status = "on"
+            if self.state == "off":
+                self.state = "on"
 
         def turn_off(self) -> None:
-            if self.status == "on":
-                self.status = "off"
+            if self.state == "on":
+                self.state = "off"
 
     class Node(SimComponent):
         name: str
@@ -141,7 +153,7 @@ def test_hierarchical_action_with_validation() -> None:
         def send_action_to_app(self, app_name: str, options: List[str], context: Dict):
             for app in self.apps:
                 if app_name == app.name:
-                    app.apply_action(options)
+                    app.apply_action(options, context)
                     break
             else:
                 msg = f"Node has no app with name {app_name}"
@@ -163,9 +175,16 @@ def test_hierarchical_action_with_validation() -> None:
         }
     }
 
+    # check that a non-admin can't disable this app
     my_node.apply_action(["apps", "Chrome", "disable"], non_admin_context)
-    my_node.apply_action(["apps", "Firefox", "turn_on"], non_admin_context)
+    assert my_node.apps[0].name == "Chrome"  # if failure occurs on this line, the test itself is broken
+    assert my_node.apps[0].state == "off"
 
-    assert my_node.apps[0].name == "Chrome"
-    assert my_node.apps[1].name == "Firefox"
-    assert my_node.apps[0].state == ...  # TODO: finish
+    # check that a non-admin can turn this app on
+    my_node.apply_action(["apps", "Firefox", "turn_on"], non_admin_context)
+    assert my_node.apps[1].name == "Firefox"  # if failure occurs on this line, the test itself is broken
+    assert my_node.apps[1].state == "on"
+
+    # check that an admin can disable this app
+    my_node.apply_action(["apps", "Chrome", "disable"], admin_context)
+    assert my_node.apps[0].state == "disabled"
diff --git a/tests/unit_tests/_primaite/_simulator/_domain/__init__.py b/tests/unit_tests/_primaite/_simulator/_domain/__init__.py
new file mode 100644
index 00000000..e69de29b
diff --git a/tests/unit_tests/_primaite/_simulator/_domain/test_account.py b/tests/unit_tests/_primaite/_simulator/_domain/test_account.py
new file mode 100644
index 00000000..d4a57179
--- /dev/null
+++ b/tests/unit_tests/_primaite/_simulator/_domain/test_account.py
@@ -0,0 +1,18 @@
+"""Test the account module of the simulator."""
+from primaite.simulator.domain.account import Account, AccountType
+
+
+def test_account_serialise():
+    """Test that an account can be serialised."""
+    acct = Account(username="Jake", password="JakePass1!", account_type=AccountType.user)
+    serialised = acct.model_dump_json()
+    print(serialised)
+
+
+def test_account_deserialise():
+    """Test that an account can be deserialised."""
+    acct_json = (
+        '{"uuid":"dfb2bcaa-d3a1-48fd-af3f-c943354622b4","num_logons":0,"num_logoffs":0,"num_group_changes":0,'
+        '"username":"Jake","password":"JakePass1!","account_type":2,"status":2,"action_manager":null}'
+    )
+    acct = Account.model_validate_json(acct_json)
diff --git a/tests/unit_tests/_primaite/_simulator/_domain/test_controller.py b/tests/unit_tests/_primaite/_simulator/_domain/test_controller.py
new file mode 100644
index 00000000..e69de29b

From 84b6e2206e362e43b344084ced42a367f89824d5 Mon Sep 17 00:00:00 2001
From: Marek Wolan <marek.wolan@methods.co.uk>
Date: Mon, 7 Aug 2023 10:18:27 +0000
Subject: [PATCH 07/16] Updated CHANGELOG.md

---
 CHANGELOG.md | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d66257b5..fbdd1d5d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+- Permission System - each agent action can define criteria that will be used to permit or deny agent actions.
+
+
 ## [2.0.0] - 2023-07-26
 
 ### Added

From 22afdc9134a5df947d12b74492e27a3c73f8502a Mon Sep 17 00:00:00 2001
From: Marek Wolan <marek.wolan@methods.co.uk>
Date: Mon, 7 Aug 2023 10:19:06 +0000
Subject: [PATCH 08/16] Updated pull_request_template.md

---
 .azuredevops/pull_request_template.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/.azuredevops/pull_request_template.md b/.azuredevops/pull_request_template.md
index fd28ed57..f7533b37 100644
--- a/.azuredevops/pull_request_template.md
+++ b/.azuredevops/pull_request_template.md
@@ -9,5 +9,6 @@
 - [ ] I have performed **self-review** of the code
 - [ ] I have written **tests** for any new functionality added with this PR
 - [ ] I have updated the **documentation** if this PR changes or adds functionality
-- [ ] I have written/updated **design docs** if this PR implements new functionality.
+- [ ] I have written/updated **design docs** if this PR implements new functionality
+- [ ] I have update the **change log**
 - [ ] I have run **pre-commit** checks for code style

From 7eb0bb428fc26eb487a50ae496b3b2b8f4640eb2 Mon Sep 17 00:00:00 2001
From: Marek Wolan <marek.wolan@methods.co.uk>
Date: Mon, 7 Aug 2023 17:24:14 +0100
Subject: [PATCH 09/16] Update code based on PR comments.

---
 src/primaite/simulator/core.py              | 38 ++++++++++++++++++---
 src/primaite/simulator/domain/account.py    | 20 +++++------
 src/primaite/simulator/domain/controller.py | 16 +++++----
 3 files changed, 54 insertions(+), 20 deletions(-)

diff --git a/src/primaite/simulator/core.py b/src/primaite/simulator/core.py
index 17e09f85..fa5cd6c7 100644
--- a/src/primaite/simulator/core.py
+++ b/src/primaite/simulator/core.py
@@ -21,7 +21,7 @@ class ActionPermissionValidator(ABC):
 
     @abstractmethod
     def __call__(self, request: List[str], context: Dict) -> bool:
-        """TODO."""
+        """Use the request and context paramters to decide whether the action should be permitted."""
         pass
 
 
@@ -52,6 +52,10 @@ class Action:
         turning it off, then the SimComponent should have a turn_off(self) method that does not need to accept any args.
         Then, this Action will be given something like ``func = lambda request, context: self.turn_off()``.
 
+        ``validator`` is an instance of a subclass of `ActionPermissionValidator`. This is essentially a callable that
+        accepts `request` and `context` and returns a boolean to represent whether the permission is granted to perform
+        the action.
+
         :param func: Function that performs the request.
         :type func: Callable[[List[str], Dict], None]
         :param validator: Function that checks if the request is authenticated given the context.
@@ -62,14 +66,28 @@ class Action:
 
 
 class ActionManager:
-    """TODO."""
+    """
+    ActionManager is used by `SimComponent` instances to keep track of actions.
+
+    Its main purpose is to be a lookup from action name to action function and corresponding validation function. This
+    class is responsible for providing a consistent API for processing actions as well as helpful error messages.
+    """
 
     def __init__(self) -> None:
-        """TODO."""
+        """Initialise ActionManager with an empty action lookup."""
         self.actions: Dict[str, Action] = {}
 
     def process_request(self, request: List[str], context: Dict) -> None:
-        """Process action request."""
+        """Process an action request.
+
+        :param request: A list of strings which specify what action to take. The first string must be one of the allowed
+            actions, i.e. it must be a key of self.actions. The subsequent strings in the list are passed as parameters
+            to the action function.
+        :type request: List[str]
+        :param context: Dictionary of additional information necessary to process or validate the request.
+        :type context: Dict
+        :raises RuntimeError: If the request parameter does not have a valid action identifier as the first item.
+        """
         action_key = request[0]
 
         if action_key not in self.actions:
@@ -90,6 +108,18 @@ class ActionManager:
         action.func(action_options, context)
 
     def add_action(self, name: str, action: Action) -> None:
+        """Add an action to this action manager.
+
+        :param name: The string associated to this action.
+        :type name: str
+        :param action: Action object.
+        :type action: Action
+        """
+        if name in self.actions:
+            msg = f"Attempted to register an action but the action name {name} is already taken."
+            _LOGGER.error(msg)
+            raise RuntimeError(msg)
+
         self.actions[name] = action
 
 
diff --git a/src/primaite/simulator/domain/account.py b/src/primaite/simulator/domain/account.py
index 086022e6..2d726624 100644
--- a/src/primaite/simulator/domain/account.py
+++ b/src/primaite/simulator/domain/account.py
@@ -11,25 +11,25 @@ _LOGGER = getLogger(__name__)
 class AccountType(Enum):
     """Whether the account is intended for a user to log in or for a service to use."""
 
-    service = 1
+    SERVICE = 1
     "Service accounts are used to grant permissions to software on nodes to perform actions"
-    user = 2
+    USER = 2
     "User accounts are used to allow agents to log in and perform actions"
 
 
 class AccountStatus(Enum):
     """Whether the account is active."""
 
-    enabled = 1
-    disabled = 2
+    ENABLED = 1
+    DISABLED = 2
 
 
 class PasswordPolicyLevel(Enum):
     """Complexity requirements for account passwords."""
 
-    low = 1
-    medium = 2
-    high = 3
+    LOW = 1
+    MEDIUM = 2
+    HIGH = 3
 
 
 class Account(SimComponent):
@@ -47,7 +47,7 @@ class Account(SimComponent):
     "Account password."
     account_type: AccountType
     "Account Type, currently this can be service account (used by apps) or user account."
-    status: AccountStatus = AccountStatus.disabled
+    status: AccountStatus = AccountStatus.DISABLED
 
     def describe_state(self) -> Dict:
         """Describe state for agent observations."""
@@ -55,11 +55,11 @@ class Account(SimComponent):
 
     def enable(self):
         """Set the status to enabled."""
-        self.status = AccountStatus.enabled
+        self.status = AccountStatus.ENABLED
 
     def disable(self):
         """Set the status to disabled."""
-        self.status = AccountStatus.disabled
+        self.status = AccountStatus.DISABLED
 
     def log_on(self) -> None:
         """TODO."""
diff --git a/src/primaite/simulator/domain/controller.py b/src/primaite/simulator/domain/controller.py
index e4a73b4e..cc8063d6 100644
--- a/src/primaite/simulator/domain/controller.py
+++ b/src/primaite/simulator/domain/controller.py
@@ -47,7 +47,11 @@ class GroupMembershipValidator(ActionPermissionValidator):
     """Permit actions based on group membership."""
 
     def __init__(self, allowed_groups: List[AccountGroup]) -> None:
-        """TODO."""
+        """Store a list of groups that should be granted permission.
+
+        :param allowed_groups: List of AccountGroups that are permitted to perform some action.
+        :type allowed_groups: List[AccountGroup]
+        """
         self.allowed_groups = allowed_groups
 
     def __call__(self, request: List[str], context: Dict) -> bool:
@@ -64,7 +68,7 @@ class DomainController(SimComponent):
     """Main object for controlling the domain."""
 
     # owned objects
-    accounts: List[Account] = []
+    accounts: Dict[str, Account] = {}
     groups: Final[List[AccountGroup]] = list(AccountGroup)
 
     domain_group_membership: Dict[Literal[AccountGroup.domain_admin, AccountGroup.domain_user], List[Account]] = {}
@@ -73,10 +77,10 @@ class DomainController(SimComponent):
     ] = {}
 
     # references to non-owned objects. Not sure if all are needed here.
-    nodes: List[temp_node] = []
-    applications: List[temp_application] = []
-    folders: List[temp_folder] = []
-    files: List[temp_file] = []
+    nodes: Dict[str, temp_node] = {}
+    applications: Dict[str, temp_application] = {}
+    folders: List[temp_folder] = {}
+    files: List[temp_file] = {}
 
     def _register_account(self, account: Account) -> None:
         """TODO."""

From 1de8e0a058d6ee1d633171b154745fc2e9024787 Mon Sep 17 00:00:00 2001
From: Marek Wolan <marek.wolan@methods.co.uk>
Date: Wed, 9 Aug 2023 09:19:11 +0100
Subject: [PATCH 10/16] Update tests

---
 .../_simulator/_domain/test_account.py        |  2 +-
 .../_primaite/_simulator/test_core.py         | 33 -------------------
 2 files changed, 1 insertion(+), 34 deletions(-)

diff --git a/tests/unit_tests/_primaite/_simulator/_domain/test_account.py b/tests/unit_tests/_primaite/_simulator/_domain/test_account.py
index d4a57179..aadf1c69 100644
--- a/tests/unit_tests/_primaite/_simulator/_domain/test_account.py
+++ b/tests/unit_tests/_primaite/_simulator/_domain/test_account.py
@@ -4,7 +4,7 @@ from primaite.simulator.domain.account import Account, AccountType
 
 def test_account_serialise():
     """Test that an account can be serialised."""
-    acct = Account(username="Jake", password="JakePass1!", account_type=AccountType.user)
+    acct = Account(username="Jake", password="JakePass1!", account_type=AccountType.USER)
     serialised = acct.model_dump_json()
     print(serialised)
 
diff --git a/tests/unit_tests/_primaite/_simulator/test_core.py b/tests/unit_tests/_primaite/_simulator/test_core.py
index 4e2df757..0d227633 100644
--- a/tests/unit_tests/_primaite/_simulator/test_core.py
+++ b/tests/unit_tests/_primaite/_simulator/test_core.py
@@ -44,36 +44,3 @@ class TestIsolatedSimComponent:
         comp = TestComponent(name="computer", size=(5, 10))
         dump = comp.model_dump_json()
         assert comp == TestComponent.model_validate_json(dump)
-
-    def test_apply_action(self):
-        """Validate that we can override apply_action behaviour and it updates the state of the component."""
-
-        class TestComponent(SimComponent):
-            name: str
-            status: Literal["on", "off"] = "off"
-
-            def describe_state(self) -> Dict:
-                return {}
-
-            def _possible_actions(self) -> Dict[str, Callable[[List[str]], None]]:
-                return {
-                    "turn_off": self._turn_off,
-                    "turn_on": self._turn_on,
-                }
-
-            def _turn_off(self, options: List[str]) -> None:
-                assert len(options) == 0, "This action does not support options."
-                self.status = "off"
-
-            def _turn_on(self, options: List[str]) -> None:
-                assert len(options) == 0, "This action does not support options."
-                self.status = "on"
-
-        comp = TestComponent(name="computer", status="off")
-
-        assert comp.status == "off"
-        comp.apply_action(["turn_on"])
-        assert comp.status == "on"
-
-        with pytest.raises(ValueError):
-            comp.apply_action(["do_nothing"])

From be8c2955ced0c41379f5cd98ac4bc3f93006cf47 Mon Sep 17 00:00:00 2001
From: Marek Wolan <marek.wolan@methods.co.uk>
Date: Wed, 9 Aug 2023 10:26:52 +0100
Subject: [PATCH 11/16] Change Accountstatus to a bool

---
 src/primaite/simulator/domain/account.py | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/src/primaite/simulator/domain/account.py b/src/primaite/simulator/domain/account.py
index 2d726624..79d0de23 100644
--- a/src/primaite/simulator/domain/account.py
+++ b/src/primaite/simulator/domain/account.py
@@ -17,13 +17,6 @@ class AccountType(Enum):
     "User accounts are used to allow agents to log in and perform actions"
 
 
-class AccountStatus(Enum):
-    """Whether the account is active."""
-
-    ENABLED = 1
-    DISABLED = 2
-
-
 class PasswordPolicyLevel(Enum):
     """Complexity requirements for account passwords."""
 
@@ -47,7 +40,7 @@ class Account(SimComponent):
     "Account password."
     account_type: AccountType
     "Account Type, currently this can be service account (used by apps) or user account."
-    status: AccountStatus = AccountStatus.DISABLED
+    enabled: bool = True
 
     def describe_state(self) -> Dict:
         """Describe state for agent observations."""
@@ -55,11 +48,11 @@ class Account(SimComponent):
 
     def enable(self):
         """Set the status to enabled."""
-        self.status = AccountStatus.ENABLED
+        self.enabled = True
 
     def disable(self):
         """Set the status to disabled."""
-        self.status = AccountStatus.DISABLED
+        self.enabled = False
 
     def log_on(self) -> None:
         """TODO."""

From 596bbaacdeb07b942a14118a09aedf452744ad32 Mon Sep 17 00:00:00 2001
From: Marek Wolan <marek.wolan@methods.co.uk>
Date: Wed, 9 Aug 2023 12:06:06 +0100
Subject: [PATCH 12/16] Change enum strings to uppercase

---
 src/primaite/simulator/domain/controller.py        | 12 ++++++------
 .../component_creation/test_permission_system.py   | 14 +++++++-------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/primaite/simulator/domain/controller.py b/src/primaite/simulator/domain/controller.py
index 4f09a846..887a065d 100644
--- a/src/primaite/simulator/domain/controller.py
+++ b/src/primaite/simulator/domain/controller.py
@@ -33,13 +33,13 @@ class temp_file:
 class AccountGroup(Enum):
     """Permissions are set at group-level and accounts can belong to these groups."""
 
-    local_user = 1
+    LOCAL_USER = 1
     "For performing basic actions on a node"
-    domain_user = 2
+    DOMAIN_USER = 2
     "For performing basic actions to the domain"
-    local_admin = 3
+    LOCAL_ADMIN = 3
     "For full access to actions on a node"
-    domain_admin = 4
+    DOMAIN_ADMIN = 4
     "For full access"
 
 
@@ -71,9 +71,9 @@ class DomainController(SimComponent):
     accounts: Dict[str, Account] = {}
     groups: Final[List[AccountGroup]] = list(AccountGroup)
 
-    domain_group_membership: Dict[Literal[AccountGroup.domain_admin, AccountGroup.domain_user], List[Account]] = {}
+    domain_group_membership: Dict[Literal[AccountGroup.DOMAIN_ADMIN, AccountGroup.DOMAIN_USER], List[Account]] = {}
     local_group_membership: Dict[
-        Tuple[temp_node, Literal[AccountGroup.local_admin, AccountGroup.local_user]], List[Account]
+        Tuple[temp_node, Literal[AccountGroup.LOCAL_ADMIN, AccountGroup.LOCAL_USER]], List[Account]
     ] = {}
 
     # references to non-owned objects. Not sure if all are needed here.
diff --git a/tests/integration_tests/component_creation/test_permission_system.py b/tests/integration_tests/component_creation/test_permission_system.py
index 93d0267c..6816ba84 100644
--- a/tests/integration_tests/component_creation/test_permission_system.py
+++ b/tests/integration_tests/component_creation/test_permission_system.py
@@ -34,7 +34,7 @@ def test_group_action_validation() -> None:
                 "create_folder",
                 Action(
                     func=lambda request, context: self.create_folder(request[0]),
-                    validator=GroupMembershipValidator([AccountGroup.local_admin, AccountGroup.domain_admin]),
+                    validator=GroupMembershipValidator([AccountGroup.LOCAL_ADMIN, AccountGroup.DOMAIN_ADMIN]),
                 ),
             )
 
@@ -49,14 +49,14 @@ def test_group_action_validation() -> None:
             self.folders = [x for x in self.folders if x is not folder]
 
     # check that the folder is created when a local admin tried to do it
-    permitted_context = {"request_source": {"agent": "BLUE", "account": "User1", "groups": ["local_admin"]}}
+    permitted_context = {"request_source": {"agent": "BLUE", "account": "User1", "groups": ["LOCAL_ADMIN"]}}
     my_node = Node(uuid="0000-0000-1234", name="pc")
     my_node.apply_action(["create_folder", "memes"], context=permitted_context)
     assert len(my_node.folders) == 1
     assert my_node.folders[0].name == "memes"
 
     # check that the number of folders is still 1 even after attempting to create a second one without permissions
-    invalid_context = {"request_source": {"agent": "BLUE", "account": "User1", "groups": ["local_user", "domain_user"]}}
+    invalid_context = {"request_source": {"agent": "BLUE", "account": "User1", "groups": ["LOCAL_USER", "DOMAIN_USER"]}}
     my_node.apply_action(["create_folder", "memes2"], context=invalid_context)
     assert len(my_node.folders) == 1
     assert my_node.folders[0].name == "memes"
@@ -97,14 +97,14 @@ def test_hierarchical_action_with_validation() -> None:
                 "disable",
                 Action(
                     func=lambda request, context: self.disable(),
-                    validator=GroupMembershipValidator([AccountGroup.local_admin, AccountGroup.domain_admin]),
+                    validator=GroupMembershipValidator([AccountGroup.LOCAL_ADMIN, AccountGroup.DOMAIN_ADMIN]),
                 ),
             )
             self.action_manager.add_action(
                 "enable",
                 Action(
                     func=lambda request, context: self.enable(),
-                    validator=GroupMembershipValidator([AccountGroup.local_admin, AccountGroup.domain_admin]),
+                    validator=GroupMembershipValidator([AccountGroup.LOCAL_ADMIN, AccountGroup.DOMAIN_ADMIN]),
                 ),
             )
 
@@ -164,14 +164,14 @@ def test_hierarchical_action_with_validation() -> None:
     my_node.install_app("Firefox")
 
     non_admin_context = {
-        "request_source": {"agent": "BLUE", "account": "User1", "groups": ["local_user", "domain_user"]}
+        "request_source": {"agent": "BLUE", "account": "User1", "groups": ["LOCAL_USER", "DOMAIN_USER"]}
     }
 
     admin_context = {
         "request_source": {
             "agent": "BLUE",
             "account": "User1",
-            "groups": ["local_admin", "domain_admin", "local_user", "domain_user"],
+            "groups": ["LOCAL_ADMIN", "DOMAIN_ADMIN", "LOCAL_USER", "DOMAIN_USER"],
         }
     }
 

From 51baabb35ba27d318db16478d140e92e6d5cb2cb Mon Sep 17 00:00:00 2001
From: Marek Wolan <marek.wolan@methods.co.uk>
Date: Wed, 9 Aug 2023 12:34:56 +0100
Subject: [PATCH 13/16] Update enums to uppercase in docs

---
 docs/source/simulation_structure.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/source/simulation_structure.rst b/docs/source/simulation_structure.rst
index 479b3e7b..2b213f16 100644
--- a/docs/source/simulation_structure.rst
+++ b/docs/source/simulation_structure.rst
@@ -49,7 +49,7 @@ snippet demonstrates usage of the ``ActionPermissionValidator``.
                 "reset_factory_settings",
                 Action(
                     func = lambda request, context: self.reset_factory_settings(),
-                    validator = GroupMembershipValidator([AccountGroup.domain_admin]),
+                    validator = GroupMembershipValidator([AccountGroup.DOMAIN_ADMIN]),
                 ),
             )
 
@@ -59,7 +59,7 @@ snippet demonstrates usage of the ``ActionPermissionValidator``.
     phone = Smartphone(name="phone1")
 
     # try to wipe the phone as a domain user, this will have no effect
-    phone.apply_action(["reset_factory_settings"], context={"request_source":{"groups":["domain_user"]})
+    phone.apply_action(["reset_factory_settings"], context={"request_source":{"groups":["DOMAIN_USER"]})
 
     # try to wipe the phone as an admin user, this will wipe the phone
-    phone.apply_action(["reset_factory_settings"], context={"request_source":{"groups":["domain_admin"]})
+    phone.apply_action(["reset_factory_settings"], context={"request_source":{"groups":["DOMAIN_ADMIN"]})

From f198a8b94d22ffa5ffd3b74bed6244105c5a8bbb Mon Sep 17 00:00:00 2001
From: Marek Wolan <marek.wolan@methods.co.uk>
Date: Wed, 9 Aug 2023 12:36:09 +0100
Subject: [PATCH 14/16] Fix bad merge

---
 src/primaite/simulator/core.py | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/src/primaite/simulator/core.py b/src/primaite/simulator/core.py
index 779358ec..caba5210 100644
--- a/src/primaite/simulator/core.py
+++ b/src/primaite/simulator/core.py
@@ -126,6 +126,8 @@ class ActionManager:
 class SimComponent(BaseModel):
     """Extension of pydantic BaseModel with additional methods that must be defined by all classes in the simulator."""
 
+    model_config = ConfigDict(arbitrary_types_allowed=True, extra=Extra.allow)
+
     uuid: str
     """The component UUID."""
 
@@ -133,13 +135,6 @@ class SimComponent(BaseModel):
         if not kwargs.get("uuid"):
             kwargs["uuid"] = str(uuid4())
         super().__init__(**kwargs)
-
-    model_config = ConfigDict(arbitrary_types_allowed=True, extra=Extra.allow)
-    uuid: str = str(uuid4())
-    "The component UUID."
-
-    def __init__(self, **kwargs) -> None:
-        super().__init__(**kwargs)
         self.action_manager: Optional[ActionManager] = None
 
     @abstractmethod

From 34ff9abd7ab8e832e1b3a2cc5e66d193f0846687 Mon Sep 17 00:00:00 2001
From: Marek Wolan <marek.wolan@methods.co.uk>
Date: Wed, 9 Aug 2023 15:55:28 +0100
Subject: [PATCH 15/16] Apply changes from code review.

---
 src/primaite/simulator/core.py                              | 1 +
 src/primaite/simulator/domain/account.py                    | 4 ++--
 .../unit_tests/_primaite/_simulator/_domain/test_account.py | 6 +++---
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/primaite/simulator/core.py b/src/primaite/simulator/core.py
index caba5210..8b771cd7 100644
--- a/src/primaite/simulator/core.py
+++ b/src/primaite/simulator/core.py
@@ -127,6 +127,7 @@ class SimComponent(BaseModel):
     """Extension of pydantic BaseModel with additional methods that must be defined by all classes in the simulator."""
 
     model_config = ConfigDict(arbitrary_types_allowed=True, extra=Extra.allow)
+    """Configure pydantic to allow arbitrary types and to let the instance have attributes not present in model."""
 
     uuid: str
     """The component UUID."""
diff --git a/src/primaite/simulator/domain/account.py b/src/primaite/simulator/domain/account.py
index 79d0de23..e8595afa 100644
--- a/src/primaite/simulator/domain/account.py
+++ b/src/primaite/simulator/domain/account.py
@@ -55,9 +55,9 @@ class Account(SimComponent):
         self.enabled = False
 
     def log_on(self) -> None:
-        """TODO."""
+        """TODO. Once the accounts are integrated with nodes, populate this accordingly."""
         self.num_logons += 1
 
     def log_off(self) -> None:
-        """TODO."""
+        """TODO. Once the accounts are integrated with nodes, populate this accordingly."""
         self.num_logoffs += 1
diff --git a/tests/unit_tests/_primaite/_simulator/_domain/test_account.py b/tests/unit_tests/_primaite/_simulator/_domain/test_account.py
index aadf1c69..3a2a5903 100644
--- a/tests/unit_tests/_primaite/_simulator/_domain/test_account.py
+++ b/tests/unit_tests/_primaite/_simulator/_domain/test_account.py
@@ -3,14 +3,14 @@ from primaite.simulator.domain.account import Account, AccountType
 
 
 def test_account_serialise():
-    """Test that an account can be serialised."""
-    acct = Account(username="Jake", password="JakePass1!", account_type=AccountType.USER)
+    """Test that an account can be serialised. If pydantic throws error then this test fails."""
+    acct = Account(username="Jake", password="JakePass1!", account_type=AccountType.user)
     serialised = acct.model_dump_json()
     print(serialised)
 
 
 def test_account_deserialise():
-    """Test that an account can be deserialised."""
+    """Test that an account can be deserialised. The test fails if pydantic throws an error."""
     acct_json = (
         '{"uuid":"dfb2bcaa-d3a1-48fd-af3f-c943354622b4","num_logons":0,"num_logoffs":0,"num_group_changes":0,'
         '"username":"Jake","password":"JakePass1!","account_type":2,"status":2,"action_manager":null}'

From e24d4b88900a6667df725c4da7f95ac71f92e42e Mon Sep 17 00:00:00 2001
From: Marek Wolan <marek.wolan@methods.co.uk>
Date: Thu, 10 Aug 2023 09:14:45 +0100
Subject: [PATCH 16/16] Fix typo in test

---
 tests/unit_tests/_primaite/_simulator/_domain/test_account.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/unit_tests/_primaite/_simulator/_domain/test_account.py b/tests/unit_tests/_primaite/_simulator/_domain/test_account.py
index 3a2a5903..b5632ea7 100644
--- a/tests/unit_tests/_primaite/_simulator/_domain/test_account.py
+++ b/tests/unit_tests/_primaite/_simulator/_domain/test_account.py
@@ -4,7 +4,7 @@ from primaite.simulator.domain.account import Account, AccountType
 
 def test_account_serialise():
     """Test that an account can be serialised. If pydantic throws error then this test fails."""
-    acct = Account(username="Jake", password="JakePass1!", account_type=AccountType.user)
+    acct = Account(username="Jake", password="JakePass1!", account_type=AccountType.USER)
     serialised = acct.model_dump_json()
     print(serialised)