refactors

PROJECT_CONTEXT.md

@@ -1,34 +1,61 @@
-CA/PKI Backend Project Context
+CA/PKI Backend Project Context (Updated)
 Stack
 
 Python 3 + psycopg (dict_row cursors)
 
 PostgreSQL database: ca
 
-Unit tests: unittest (python3 -m unittest discover)
+Unit tests: unittest
+
+Run via: python3 -m unittest discover
+
+Current test count: 17
 
 Database Schema (current assumptions)
 entity
 
-id INT identity PK
+id INT GENERATED ALWAYS AS IDENTITY PRIMARY KEY
 
-creation_ts TIMESTAMPTZ default now()
+creation_ts TIMESTAMPTZ DEFAULT now()
 
-creator INT FK → entity(id) (the entity that created this one; nullable)
+creator INT FK → entity(id) (nullable)
 
 name VARCHAR(100) NOT NULL
 
-type VARCHAR(...) NOT NULL (e.g. person, group, device, alias)
+type VARCHAR(...) NOT NULL
+
+Allowed types: person, group, device
 
 public_key VARCHAR(300) NOT NULL
 
 symmetrical_key VARCHAR(100) NULL
 
-status VARCHAR(...) NOT NULL default 'active' (values: 'active', 'revoked')
+status VARCHAR(...) NOT NULL DEFAULT 'active'
+
+Values: 'active', 'revoked'
 
 expiration DATE NULL
 
-Index on entity(name) (and other indexes as needed)
+ca_reference VARCHAR(100) NULL
+
+Constraint
+CHECK (
+    (type = 'group' AND ca_reference IS NOT NULL)
+    OR
+    (type <> 'group' AND ca_reference IS NULL)
+)
+
+Rule:
+
+Groups MUST have a ca_reference
+
+All other entity types MUST have ca_reference IS NULL
+
+Indexes:
+
+Index on entity(name)
+
+Other indexes as needed
 
 group_member
 
@@ -38,69 +65,169 @@ member_id INT FK → entity(id) ON DELETE CASCADE
 
 role VARCHAR(10)
 
-PK (group_id, member_id)
+PRIMARY KEY (group_id, member_id)
 
 Index (member_id, group_id)
 
-Groups can contain any entity type, including other groups and devices.
+Groups can contain:
+
+persons
+
+devices
+
+other groups
 
 property
 
-Columns: (id INT FK → entity(id), property_name VARCHAR(100))
+id INT FK → entity(id) ON DELETE CASCADE
 
-PK (id, property_name)
+property_name VARCHAR(100) NOT NULL
 
-Used for flags/roles such as "creator"
+validation_policy CHAR(19) NOT NULL DEFAULT 'default'
+
+source VARCHAR(150) NULL
+
+PRIMARY KEY (id, property_name)
+
+Notes:
+
+validation_policy is CHAR(19) and padded by PostgreSQL.
+
+Used for flags/roles such as "creator".
 
 metadata
 
-Intended “singleton row” table, enforced at application level
+Singleton row table (enforced at application level).
 
-Columns: name, comment, private_key, public_key
+Columns:
+
+name
+
+comment
+
+private_key
+
+public_key
+
+defense_p BOOLEAN NOT NULL DEFAULT false
+
+defense_p:
+
+Global system flag.
+
+Logged on change.
 
 log
 
-id SERIAL PK
+id SERIAL PRIMARY KEY
 
-ts TIMESTAMPTZ default now()
+ts TIMESTAMPTZ DEFAULT now()
 
 entry TEXT NOT NULL
 
-Every API mutation must log one row here.
+Every API mutation must insert exactly one row here.
 
 Core Business Rules
+Creators are NOT an entity type
 
-Creators are not an entity type
+"creator" is a property (property_name='creator') on a person.
 
-creator is a property (property_name='creator') on a person entity.
+insert_creator():
 
-insert_creator() creates a person entity and inserts the creator property.
+Creates a person
+
+Adds "creator" property
 
 Revoked entities are immutable
 
-Any mutation on an entity requires ensure_entity_active(cursor, entity_id).
+All entity mutations must call:
 
-Revoked entities cannot:
+ensure_entity_active(cursor, entity_id)
 
-Join groups or accept members
+Revoked entities CANNOT:
+
+Join groups
+
+Accept members
 
 Add/delete properties
 
-Change keys (public_key, symmetrical_key)
+Change public_key
+
+Change symmetrical_key
 
 Change status again
 
-Logging
+Group CA Reference Rule
 
-All changes to entity, group_member, property, metadata must call log_change(cursor, "...")
+group entities must include a non-null ca_reference.
 
-Logging happens inside the same transaction (no extra commits).
+person and device must not define ca_reference.
 
-Python Modules (current structure)
+Enforced at:
 
+Database level (CHECK constraint)
+
+Python validation level
+
+Property Metadata
+
+Each property includes:
+
+validation_policy
+
+source
+
+Defaults:
+
+validation_policy = 'default'
+
+source = NULL
+
+Property mutations:
+
+Require entity to be active
+
+Must log changes
+
+Metadata Defense Flag
+
+defense_p:
+
+Boolean system-wide flag
+
+Default: false
+
+Must be logged when changed
+
+Logging Rules
+
+All changes to:
+
+entity
+
+group_member
+
+property
+
+metadata
+
+Must call:
+
+log_change(cursor, "...")
+
+Logging:
+
+Happens inside same transaction
+
+No extra commits
+
+Exactly one log row per mutation
+
+Python Modules
 ca_core/entity.py
 
-Must provide:
+Provides:
 
 ensure_entity_active(cursor, entity_id)
 
@@ -108,60 +235,94 @@ insert_creator(cursor, name, public_key)
 
 enroll_person(cursor, name, public_key, creator_id)
 
-create_group(cursor, name, public_key, creator_id)
-
-create_alias(cursor, target_entity_id)
+create_group(cursor, name, public_key, creator_id, ca_reference)
 
 get_entity(cursor, entity_id)
 
-set_entity_status(cursor, entity_id, status, changed_by) (requires active entity)
+set_entity_status(cursor, entity_id, status, changed_by)
 
-set_entity_keys(cursor, entity_id, public_key, changed_by) (active-only)
+set_entity_keys(cursor, entity_id, public_key, changed_by)
 
-set_symmetrical_key(cursor, entity_id, key, changed_by) (active-only)
+set_symmetrical_key(cursor, entity_id, key, changed_by)
 
 get_symmetrical_key(cursor, entity_id)
 
 ca_core/group_member.py
 
-Uses member_id (not person_id)
+Uses member_id
 
-Must prevent adding revoked groups/members (via ensure_entity_active)
+Prevents adding revoked groups/members
 
-Logs add/remove membership
+Logs membership add/remove
 
 ca_core/property.py
 
-Table is property(id, property_name) (NOT entity_id/name)
+Table:
 
-Must reject mutations if entity revoked (immutability)
+property(id, property_name, validation_policy, source)
 
-Logs set/delete property
+Rules:
+
+Reject mutations if entity revoked
+
+Logs set/delete
+
+Default policy 'default'
+
+validation_policy is CHAR(19)
 
 ca_core/metadata.py
 
-Updates metadata fields and logs changes
+Updates metadata fields
+
+Manages defense_p
+
+Logs changes
 
 ca_core/db_logging.py
+log_change(cursor, message: str)
 
-log_change(cursor, message: str) inserts into log(entry)
+Inserts into log(entry).
 
 Tests
 
-tests/test_entity.py, tests/test_group.py, tests/test_property.py, tests/test_metadata.py
+tests/test_entity.py
+
+tests/test_group.py
+
+tests/test_property.py
+
+tests/test_metadata.py
 
 Tests verify:
 
-Core behaviors (create, enroll, group membership, revoke immutability)
+Creation and enrollment
 
-Log entry is created for mutations (case-insensitive substring checks)
+Group membership
+
+Revocation immutability
+
+CA reference enforcement
+
+Property metadata fields
+
+defense_p behavior
+
+Log entry creation (case-insensitive substring checks)
 
 Run via:
 
 python3 -m unittest discover
+Known Gotchas
 
-Known gotchas
+Do NOT name module logging.py (conflicts with stdlib)
 
-Avoid naming a module logging.py (conflicts with stdlib). Use db_logging.py.
+Schema and code must stay aligned:
 
-Schema and code must stay aligned (e.g., property.id/property_name, group_member.member_id).
+property.id (NOT entity_id)
+
+group_member.member_id
+
+entity.ca_reference constraint
+
+CHAR(19) pads values with spaces

ca_core/entity.py

@@ -4,6 +4,7 @@ from db_logging import log_change
 def ensure_entity_active(cursor, entity_id):
     """
     Ensure an entity exists and is active.
+
     Revoked entities are immutable.
     """
     cursor.execute("SELECT status FROM entity WHERE id = %s", (entity_id,))
@@ -25,8 +26,43 @@ def _validate_ca_reference_for_group(ca_reference):
         raise ValueError("ca_reference must be at most 100 characters")
 
 
+def is_creator(cursor, entity_id):
+    """
+    Return True if the entity is a creator.
+
+    A creator is:
+      - an entity of type 'person'
+      - with a row in property where property_name = 'creator'
+    """
+    cursor.execute("SELECT type FROM entity WHERE id = %s", (entity_id,))
+    row = cursor.fetchone()
+    if row is None:
+        return False
+    if row["type"] != "person":
+        return False
+
+    cursor.execute(
+        "SELECT 1 FROM property WHERE id = %s AND property_name = %s",
+        (entity_id, "creator"),
+    )
+    return cursor.fetchone() is not None
+
+
+def ensure_creator(cursor, creator_id):
+    """
+    Ensure creator_id exists, is active, and references a creator.
+
+    A creator is a 'person' entity that has the 'creator' property.
+    """
+    ensure_entity_active(cursor, creator_id)
+    if not is_creator(cursor, creator_id):
+        raise ValueError("creator_id must reference a creator")
+
+
 def insert_creator(cursor, name, public_key):
     """
+    Create a creator.
+
     Creators are persons with property 'creator' in the property table.
     """
     cursor.execute(
@@ -54,7 +90,12 @@ def insert_creator(cursor, name, public_key):
 
 
 def enroll_person(cursor, name, public_key, creator_id):
-    ensure_entity_active(cursor, creator_id)
+    """
+    Enroll a new person under a creator.
+
+    creator_id must refer to an active creator (person + 'creator' property).
+    """
+    ensure_creator(cursor, creator_id)
 
     cursor.execute(
         """
@@ -71,7 +112,13 @@ def enroll_person(cursor, name, public_key, creator_id):
 
 
 def create_group(cursor, name, public_key, creator_id, ca_reference):
-    ensure_entity_active(cursor, creator_id)
+    """
+    Create a group under a creator.
+
+    creator_id must refer to an active creator (person + 'creator' property).
+    Groups must define a non-empty ca_reference.
+    """
+    ensure_creator(cursor, creator_id)
     _validate_ca_reference_for_group(ca_reference)
 
     cursor.execute(
@@ -98,6 +145,8 @@ def get_entity(cursor, entity_id):
 
 def set_entity_status(cursor, entity_id, status, changed_by):
     """
+    Update entity status.
+
     Only active entities can change status. Once revoked, immutable.
     """
     ensure_entity_active(cursor, entity_id)

ca_core/group_member.py

@@ -2,32 +2,67 @@ from db_logging import log_change
 from entity import ensure_entity_active
 
 
+def _get_entity_type(cursor, entity_id):
+    cursor.execute("SELECT type FROM entity WHERE id = %s", (entity_id,))
+    row = cursor.fetchone()
+    return row["type"] if row else None
+
+
+def _validate_role(role):
+    if not isinstance(role, str):
+        raise TypeError("role must be a string")
+    r = role.strip()
+    if not r:
+        raise ValueError("role must be a non-empty string")
+    if len(r) > 10:
+        raise ValueError("role must be at most 10 characters")
+    return r
+
+
 def add_group_member(cursor, group_id, member_id, role):
+    """Add a member to a group.
+
+    Rules:
+      - group_id must reference an active entity of type 'group'
+      - member_id must reference an active entity (person/device/group)
+      - role must be a non-empty string with max length 10
+      - duplicates are rejected
+    """
     ensure_entity_active(cursor, group_id)
     ensure_entity_active(cursor, member_id)
 
+    if _get_entity_type(cursor, group_id) != "group":
+        raise ValueError("group_id must reference an entity of type 'group'")
+
+    r = _validate_role(role)
+
+    cursor.execute(
+        "SELECT 1 FROM group_member WHERE group_id = %s AND member_id = %s",
+        (group_id, member_id),
+    )
+    if cursor.fetchone() is not None:
+        raise ValueError("Member is already in the group")
+
     cursor.execute(
         """
         INSERT INTO group_member (group_id, member_id, role)
         VALUES (%s, %s, %s)
         """,
-        (group_id, member_id, role)
+        (group_id, member_id, r),
     )
 
-    log_change(
-        cursor,
-        f"Added member {member_id} to group {group_id} as {role}"
-    )
+    log_change(cursor, f"Added member {member_id} to group {group_id} as {r}")
 
 
 def get_members_of_group(cursor, group_id):
+    ensure_entity_active(cursor, group_id)
     cursor.execute(
         """
         SELECT member_id, role
         FROM group_member
         WHERE group_id = %s
+        ORDER BY member_id
         """,
-        (group_id,)
+        (group_id,),
     )
     return cursor.fetchall()
-

ca_core/metadata.py

@@ -1,12 +1,29 @@
 from db_logging import log_change
 
 
+def _ensure_singleton_row(cursor):
+    """Ensure exactly one metadata row exists.
+
+    The metadata table is treated as a singleton at the application level.
+    Setters must ONLY update the relevant column(s) and must not wipe others.
+
+    If the table is empty, a single default row is inserted.
+    If the table contains more than one row, we raise to avoid ambiguous reads.
+    """
+    cursor.execute("SELECT COUNT(*) AS cnt FROM metadata")
+    row = cursor.fetchone()
+    cnt = int(row["cnt"]) if row and row["cnt"] is not None else 0
+
+    if cnt == 0:
+        # Rely on column defaults (e.g., defense_p default false) and nullable columns.
+        cursor.execute("INSERT INTO metadata DEFAULT VALUES")
+    elif cnt > 1:
+        raise ValueError("metadata table must contain exactly one row")
+
+
 def set_name(cursor, name):
-    cursor.execute("DELETE FROM metadata")
-    cursor.execute(
-        "INSERT INTO metadata (name) VALUES (%s)",
-        (name,)
-    )
+    _ensure_singleton_row(cursor)
+    cursor.execute("UPDATE metadata SET name = %s", (name,))
     log_change(cursor, f"Updated metadata name to {name}")
 
 
@@ -17,11 +34,8 @@ def get_name(cursor):
 
 
 def set_comment(cursor, comment):
-    cursor.execute("DELETE FROM metadata")
-    cursor.execute(
-        "INSERT INTO metadata (comment) VALUES (%s)",
-        (comment,)
-    )
+    _ensure_singleton_row(cursor)
+    cursor.execute("UPDATE metadata SET comment = %s", (comment,))
     log_change(cursor, f"Updated metadata comment to {comment}")
 
 
@@ -32,13 +46,10 @@ def get_comment(cursor):
 
 
 def set_keys(cursor, public_key, private_key):
-    cursor.execute("DELETE FROM metadata")
+    _ensure_singleton_row(cursor)
     cursor.execute(
-        """
-        INSERT INTO metadata (public_key, private_key)
-        VALUES (%s, %s)
-        """,
-        (public_key, private_key)
+        "UPDATE metadata SET public_key = %s, private_key = %s",
+        (public_key, private_key),
     )
     log_change(cursor, "Updated metadata keys")
 
@@ -59,13 +70,10 @@ def set_defense_p(cursor, defense_p: bool):
     """Set the metadata defense_p flag.
 
     This table is treated as a singleton row at the application level.
-    Current convention in this codebase is to wipe and re-insert.
+    This setter updates ONLY defense_p and preserves all other columns.
     """
-    cursor.execute("DELETE FROM metadata")
-    cursor.execute(
-        "INSERT INTO metadata (defense_p) VALUES (%s)",
-        (defense_p,)
-    )
+    _ensure_singleton_row(cursor)
+    cursor.execute("UPDATE metadata SET defense_p = %s", (defense_p,))
     log_change(cursor, f"Updated metadata defense_p to {defense_p}")
 
 

ca_core/property.py

@@ -68,7 +68,7 @@ def get_properties(cursor, entity_id):
     Returns a list of property_name values for the entity.
     """
     cursor.execute(
-        "SELECT property_name FROM property WHERE id = %s",
+        "SELECT property_name FROM property WHERE id = %s ORDER BY property_name",
         (entity_id,),
     )
     rows = cursor.fetchall()

tests/test_entity.py

@@ -82,3 +82,32 @@ class TestEntityFunctions(unittest.TestCase):
 
         log_entry = get_last_log(self.cur).lower()
         self.assertIn("symmetrical_key", log_entry)
+
+
+    def test_is_creator_true(self):
+        creator_id = entity.insert_creator(self.cur, "CreatorCheck", "pubkeyX")
+        self.assertTrue(entity.is_creator(self.cur, creator_id))
+
+    def test_is_creator_false_for_normal_person(self):
+        creator_id = entity.insert_creator(self.cur, "CreatorBase", "pubkeyBase")
+        person_id = entity.enroll_person(self.cur, "NormalPerson", "pubkeyP", creator_id)
+        self.assertFalse(entity.is_creator(self.cur, person_id))
+
+    def test_is_creator_false_for_group(self):
+        creator_id = entity.insert_creator(self.cur, "CreatorGroup", "pubkeyG")
+        group_id = entity.create_group(self.cur, "GroupX", "pubkeyGX", creator_id, "CA-REF-X")
+        self.assertFalse(entity.is_creator(self.cur, group_id))
+
+    def test_enroll_person_requires_creator(self):
+        creator_id = entity.insert_creator(self.cur, "CreatorY", "pubkeyY")
+        person_id = entity.enroll_person(self.cur, "PersonZ", "pubkeyZ", creator_id)
+
+        with self.assertRaises(ValueError):
+            entity.enroll_person(self.cur, "IllegalEnroll", "pubkeyBad", person_id)
+
+    def test_create_group_requires_creator(self):
+        creator_id = entity.insert_creator(self.cur, "CreatorZ", "pubkeyZ")
+        person_id = entity.enroll_person(self.cur, "PersonA", "pubkeyA", creator_id)
+
+        with self.assertRaises(ValueError):
+            entity.create_group(self.cur, "BadGroup", "pubkeyBG", person_id, "CA-REF-BAD")

tests/test_group.py

@@ -86,3 +86,33 @@ class TestGroupFunctions(unittest.TestCase):
 
         with self.assertRaises(ValueError):
             group_member.add_group_member(self.cur, group_id, person_id, "member")
+
+
+    def test_group_id_must_be_group(self):
+        creator_id = entity.insert_creator(self.cur, "CreatorType", "pubkeyT")
+        person_id = entity.enroll_person(self.cur, "PersonType", "pubkeyPT", creator_id)
+        other_person = entity.enroll_person(self.cur, "PersonType2", "pubkeyPT2", creator_id)
+
+        with self.assertRaises(ValueError):
+            group_member.add_group_member(self.cur, person_id, other_person, "member")
+
+    def test_role_validation(self):
+        creator_id = entity.insert_creator(self.cur, "CreatorRole", "pubkeyR")
+        group_id = entity.create_group(self.cur, "GroupRole", "pubkeyGR", creator_id, "CA-ROLE")
+        person_id = entity.enroll_person(self.cur, "PersonRole", "pubkeyPR", creator_id)
+
+        with self.assertRaises(ValueError):
+            group_member.add_group_member(self.cur, group_id, person_id, "")
+
+        with self.assertRaises(ValueError):
+            group_member.add_group_member(self.cur, group_id, person_id, "x" * 11)
+
+    def test_duplicate_membership_rejected(self):
+        creator_id = entity.insert_creator(self.cur, "CreatorDup", "pubkeyD")
+        group_id = entity.create_group(self.cur, "GroupDup", "pubkeyGD", creator_id, "CA-DUP")
+        person_id = entity.enroll_person(self.cur, "PersonDup", "pubkeyPD", creator_id)
+
+        group_member.add_group_member(self.cur, group_id, person_id, "member")
+
+        with self.assertRaises(ValueError):
+            group_member.add_group_member(self.cur, group_id, person_id, "member")

tests/test_metadata.py

@@ -64,3 +64,11 @@ class TestMetadataFunctions(unittest.TestCase):
 
         log_entry = get_last_log(self.cur).lower()
         self.assertIn("defense_p", log_entry)
+
+    def test_metadata_setters_do_not_wipe_other_fields(self):
+        metadata.set_name(self.cur, "PreserveMe")
+        metadata.set_defense_p(self.cur, True)
+
+        # defense_p should be updated, and name should still be present.
+        self.assertTrue(metadata.get_defense_p(self.cur))
+        self.assertEqual(metadata.get_name(self.cur), "PreserveMe")