diff --git a/PROJECT_CONTEXT.md b/PROJECT_CONTEXT.md index c2bc6a0..995e8d4 100644 --- a/PROJECT_CONTEXT.md +++ b/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 diff --git a/ca_core/__pycache__/entity.cpython-313.pyc b/ca_core/__pycache__/entity.cpython-313.pyc index 78268d1..782ccd8 100644 Binary files a/ca_core/__pycache__/entity.cpython-313.pyc and b/ca_core/__pycache__/entity.cpython-313.pyc differ diff --git a/ca_core/__pycache__/group_member.cpython-313.pyc b/ca_core/__pycache__/group_member.cpython-313.pyc index ed77cd0..3ecf616 100644 Binary files a/ca_core/__pycache__/group_member.cpython-313.pyc and b/ca_core/__pycache__/group_member.cpython-313.pyc differ diff --git a/ca_core/__pycache__/metadata.cpython-313.pyc b/ca_core/__pycache__/metadata.cpython-313.pyc index 69bac03..6f362b6 100644 Binary files a/ca_core/__pycache__/metadata.cpython-313.pyc and b/ca_core/__pycache__/metadata.cpython-313.pyc differ diff --git a/ca_core/__pycache__/property.cpython-313.pyc b/ca_core/__pycache__/property.cpython-313.pyc index c414d69..5bf21dd 100644 Binary files a/ca_core/__pycache__/property.cpython-313.pyc and b/ca_core/__pycache__/property.cpython-313.pyc differ diff --git a/ca_core/entity.py b/ca_core/entity.py index 8c565be..b1e0b07 100644 --- a/ca_core/entity.py +++ b/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) diff --git a/ca_core/group_member.py b/ca_core/group_member.py index c00cb63..d7a108c 100644 --- a/ca_core/group_member.py +++ b/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() - diff --git a/ca_core/metadata.py b/ca_core/metadata.py index cf301a5..c1c50c6 100644 --- a/ca_core/metadata.py +++ b/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}") diff --git a/ca_core/property.py b/ca_core/property.py index bf52d73..10d65b7 100644 --- a/ca_core/property.py +++ b/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() diff --git a/tests/__pycache__/test_entity.cpython-313.pyc b/tests/__pycache__/test_entity.cpython-313.pyc index f39ebef..c72f341 100644 Binary files a/tests/__pycache__/test_entity.cpython-313.pyc and b/tests/__pycache__/test_entity.cpython-313.pyc differ diff --git a/tests/__pycache__/test_group.cpython-313.pyc b/tests/__pycache__/test_group.cpython-313.pyc index d411805..6861895 100644 Binary files a/tests/__pycache__/test_group.cpython-313.pyc and b/tests/__pycache__/test_group.cpython-313.pyc differ diff --git a/tests/__pycache__/test_metadata.cpython-313.pyc b/tests/__pycache__/test_metadata.cpython-313.pyc index f93a2b3..5e7fde9 100644 Binary files a/tests/__pycache__/test_metadata.cpython-313.pyc and b/tests/__pycache__/test_metadata.cpython-313.pyc differ diff --git a/tests/test_entity.py b/tests/test_entity.py index a0bb359..304cc7e 100644 --- a/tests/test_entity.py +++ b/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") diff --git a/tests/test_group.py b/tests/test_group.py index 3452ad7..64c5673 100644 --- a/tests/test_group.py +++ b/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") diff --git a/tests/test_metadata.py b/tests/test_metadata.py index 3ac95c2..2049cac 100644 --- a/tests/test_metadata.py +++ b/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")