oehf / ipf

Open eHealth Integration Platform
http://oehf.github.io/ipf-docs
Apache License 2.0
174 stars 64 forks source link

ITI-9 and ITI-21 audit is incomplete #124

Closed ohr closed 6 years ago

ohr commented 8 years ago

Reported bug:

According to specification of audit message for ITI-21 transaction, which is emitted by Patient Demographics Supplier, the found patient result list is part of the audit message. But, the audit message emitted by the IPF pdq-iti21 consumer does not contain this result list. It is still a valid audit event (specified 0..n patients), thus not detected by audit event validators.

Quick look into IPF source code revealed there might be a problem in class org.openehealth.ipf.platform.camel.ihe.mllp.core.intercept.AuditInterceptorUtils, see lines 75/76

unixoid commented 8 years ago

Please take a look at https://groups.google.com/forum/#!searchin/ititech/pdq$20audit|sort:relevance/ititech/sj6AzTKIrgs/7BH0PbevD-IJ

ohr commented 8 years ago

I found the referenced CP at ftp://ftp.ihe.net/IT_Infrastructure/TF_Maintenance-2004-2014_Archive/TF_Maintenance-2007-2008/CPs/FinalText/Integrated/CP-ITI-342-FT.doc. But it does not reveal further insight regarding this isuue.

Is there anywhere some manifestation of the "rule" that only patient IDs in requests are audited? All I know is that a various Connect-a-thons nobody complained about missing IDs from the response.

unixoid commented 8 years ago

I have not seen any official rules. We could try to "enforce" one by reanimating the referenced discussion in ititech@.

ohr commented 8 years ago

There is some suspicious code in Iti9AuditStrategyUtils and PdqAuditStrategyUtils that show that once we at least intended to log patient IDs from the response...

OK, as long there is no clarification on this, I will leave everything as it is.

unixoid commented 8 years ago

Do you mean that those "intentions" do not collect patient IDs form responses?

ohr commented 8 years ago

No. These methods extract patient IDs from responses and put them into the Audit Dataset. But right now they are called (indirectly) using the request message (AuditInterceptorUtils, line 75/76), so these methods are simply not able to do so.

unixoid commented 8 years ago

Independently from whether IHE does or does not prescribe to include patient IDs from responses into the audit trail: the application of enrichAuditDatasetFromResponse to the request message is an unwanted side effect of aba9381e1d75e616c439948565d4a067c3d22511, where a new variable has been introduced, but not consistently propagated. I fix it just to return harmony into the world.

And I posted a question to ititech@.

ohr commented 6 years ago

Table 3.20.4.1.1.1-1, Query-Information:

"The general guidance is to log the query event with the query parameters and not the result of the query. The result of a query may be very large and is likely to be of limited value vs. the overhead. The query parameters can be used effectively to detect bad behavior and the expectation is that given the query parameters the result could be regenerated if necessary. "

DICOM PS 3.15, A 5.3.10: "The message does not record the response to the query, but merely records the fact that a query was issued"

Any objections to close this as "won't fix"?

unixoid commented 6 years ago

OK for me.

ohr commented 6 years ago

Although DICOM is clear about this, reality might be different, e.g. due to legal requirements that force patient IDs that leave a server system to be audited . I therefore added a flag to the AuditContext (created as part of #177) that is used to toggle this. By default, it is set to false, i.e. no response details are audited. For now, the flag is considered by all PIX Query/PDQ - like transactions.