digdir / dialogporten

Dialogporten - common API and and metadata state store for digital dialogs
https://docs.altinn.studio/dialogporten
MIT License
1 stars 3 forks source link

Constrain RelatedTransmissionId hierarchy #1225

Closed elsand closed 1 week ago

elsand commented 2 months ago

Introduction

This is the implementation of the concerns addressed in #1196; ie. limiting how transmissions can related to one another. With this change, a transmission can only have one single transmission pointing to it (ie. linked list semantics)

Description

The current implementation allows for any directed cyclic graphs to be represented, which is hard to reason about and even harder to create visualizations for (in arbeidsflate).

It is assumed that this flexibility is not required given the functionality that transmissions is used for, so this issue will constrain the use of relatedTransmissionId to adhere to linked list sematics without circular references. In addition, we add a depth limitiation (sufficient for any realistic use case).

Implementation

### Tasks
- [x] Change validation
- [x] Prepare documentation
- [x] Add (e2e-)tests
### Threat modelling
- [x] Does this change introduce any potential security issues? No

Acceptance criteria

These criteria apply for both dialog creation (where only the transmissions included in the request are to be validated) and dialog updates (where both the transmissions in the request and any pre-existing transmissions are to be considered for validation)

GIVEN a transmission with an existing relatedTransmissionId WHEN another transmission tries to set the same relatedTransmissionId THEN the system should reject the request, indicating that only one transmission can point to the same relatedTransmissionId

GIVEN a transmission chain with no circular references WHEN an attempt is made to create a circular reference by linking a transmission back to one of its ancestors THEN the system should reject the request and return an error indicating that circular references are not allowed

GIVEN a transmission chain with fewer than 100 transmissions WHEN a new transmission is added to extend the chain THEN the system should allow the new transmission to be added as long as the 100 transmission limit is not exceeded

GIVEN a transmission chain with 100 transmissions WHEN a new transmission is added that exceeds the depth limitation THEN the system should reject the request and return an error indicating that the transmission chain depth cannot exceed 100

LeifHelstad commented 1 week ago

Test: Første akseptkritere er testet ved at to nye transmisjoner angir samme Parent som referencedTransmissionId. Da blir den andre som forsøker avvist. "Hierarchy width violation found. 'DialogTransmission' with the following ids has to many referring DialogTransmission, exceeding the max allowed width of 1: [019353a4-585a-72ef-897e-a6dd4accd1f0]."

Andre akseptkritere krever at serviceowner sender inn to nye transmisjoner samtidig som peker på hverandre. Dette krever at serviceowner predefinerer id-ene slik at en sirkuler sammenheng finnes i det ene og samme dialog PUT kallet. "Hierarchy cyclic reference violation found. DialogTransmission with the following ids is part of a cyclic reference chain: [01935387-0dbd-7209-88f5-c37b97723c61,01935385-9db0-72a4-9152-dde53f489aea]."

Tredje akseptkritere er testet ved at det legges til en ny transmisjon i en kjede som et sted mellom 1 og 98 lang fra før. Dette er testet samtidig som første akseptkrterie sin første ekstra transmisjon ble testet. Da ble det lagt til en ny transmisjon i en kjede som var en lang fra før. (Har også testet en som ble tre lang, alle disse er mindre enn 100 og dekker kriteriet).

Fjerde akseptkriterie dekker et tilfelle der man legger til en transmisjon i en kjede som er 100 (eller mer?) fra før, det skal man nektes. Det utføres ingen dynamisk grenseverdi test av transmission chain depth i denne manuelle gjennomkjøringen. Det er gjort manuell inspeksjsjon av koden at grenseverdien ved både Create og Update er satt til 100.