ihe / iti.basicaudit Goto Github PK

Section 1:52.4.1.4 Query Parameter Handling speaks of search use cases only, but does not speak of queries in other context (e.g., Authorization Requests)

Known issues with slicing an extension in a slice -- OtherId.

Need help from Grahame, that would inform an update to Sushi

committee approval to move to TI

For RESTful Search/Query, the cleaned search parameters can be put into entity.description. Is this useful?

https://profiles.ihe.net/ITI/basicaudit/testplan.html#cucumber-actions has a "TODO…." that's not a Note as is how it's shown in other sections.

While not technically wrong, having a link to a section that's one paragraph up, seems unnecessary. A simple reference to the best-effort section above would suffice.

MustSupport - only has a meaning on items that are minimal cardinality of zero (0), and applies only to the source actor populating the data. The source actor shall populate the elements marked with MustSupport, if the concept is supported by the actor, a value exists, and security and consent rules permit. Note that sometimes MustSupport will appear on elements with a cardinality greater than zero (0), this is due to inheritance from a less constrained profile. The consuming actors should handle these elements being populated or being absent/empty. Note that sometimes MustSupport will appear on elements with a cardinality greater than zero (0), this is due to inheritance from a less constrained profile.

Is the use of AssuranceLevel proper? Should the extension element be defined more specific to NIST-800-63 assurance levels, and not allow to be carrying historical vocabulary that is not specifically assurance-level but rather the method of authentication used (e.g. urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport)?

See 3:5.7.6 Consent Authoried Decision Audit Message

Section: 1:52.1.1 Actor Descriptions and Actor Profile Requirements refers to the ATX: FHIR Feed Option without reference to what that is or where to find it. That should be added as a link to the PDF.

leave unconstrained, and add a short / or comment "not used"

Should a minimally populated or maximum populated AuditEvent have a defined .meta.security code so as to be more able to be access controlled at the Audit Consumer API?

In section 1:52.4.2.2 Use Case #2: Security token enhancement pattern, the sentence "Define how an activity AuditEvent (see Use Case #1) would be enhanced to record details from the OAuth or SAML token." looks strange to me.

This section describes CRUDE as RESTful activities as opposed to data operations. Most would assume that http commands such as POST, GET, etc. would be the closest to RESTful activities. I would suggest that this section be edited to clarify that these operations can be triggered by RESTful transactions but are not RESTful in and of themselves.

Note: Support for HL7 Security for Scalable Registration, Authentication, and Authorization (aka UDAP) when it gets published

Some SNOMED codes are used in the Disclosure profile and example. Should we get these approved, or define our own codes? Are there other available codes to use?

In Section 1:52.1 BasicAudit Actors and Content Note that the actors in this profile are the same as the actors defined in the ATNA Profile [ITI TF-1: 9] and Add RESTful ATNA (Query and Feed) Supplement.

A quick cross-check with ATNA and RESTful ATNA actors show that the actor names are not the same.

mostly with subtype
add a macro rules.fsh

There are no named options. It might be useful to have named options to enable minimal vs comprehensive; or to allow claims of compliance to only disclosure auditEvent.

In 1:52.1.1.1 ANY Secure Client

"The double recording will be different as the the AuditEvent.source would identify the actor recording the event."

I guess it should read "The double recorded messages in general will be different as the the AuditEvent.source would identify the actor recording the event."

This IG is more an abstract pattern setting than it is an implementable and conformance. Not clear how conformance to this IG will be declared in a way that is understandable at the IHE Integration Statement. Do we need named options for each defined pattern? Are the current “ANY Secure Client” and “ANY Secure Server” sufficient?

Not that this name is bad, but IHE tends to want 3 or 4 letter acronyms

• Basic Audit Accounting for Mobile - BAAm
• Basic Audit Log Pattern - BALP
• Basic Auditing - BA

note that BasicAudit had to be lowercased in the canonical URI due to a bug in the IG publisher (go-publish).

X-Request-Id header – I explained this only inside of the RESTful section, but it is applicable anywhere that X-Request-Id is used. X-Reqeust-Id is profiled differently than the example given in the FHIR core specification. Specifically there is a entity type defined here to enable slicing, where the example in FHIR core uses both type (job) and role (stream) which is harder to slice. I did not make this a standalone section because it is simply too small.
X-Request-Id vs TraceId – I added X-Request-Id profiling. I did not add TraceId, as I am not as aware of what this is. It seems very similar, unclear if it is different or the same thing. The modeling of TraceId that is in the FHIR Core is a bit different than I modeled X-Request-Id here. TraceId example in core is a .entity.type #21 “Job”, with a .entity.what.identifier.type #TraceId. Where as for X-Request-Id I followed the example that Grahame indicted his server supports today for X-Request-Id. I welcome comment, as I am not an expert TraceId nor X-Reqeust-Id.

In an AuditEvent that is describing a network interaction, should response time be recorded in the .period element? Who would be responsible for recording this response time? This seems too undefinable in abstract actor terms. Thus should it be given as guidance without constraints or requirements?

http://build.fhir.org/ig/IHE/ITI.BasicAudit/branches/main/index.html#patient-demographics-query-for-mobile

Title is Patient Demographics Query for Mobile, but should be probably: Basic Audit Log Patterns (BasicAudit) Content Profile

Priority:

• Low: Typo or other minor classification that an editor can manage. Requires no group discussion.

3:5.7.7 Privacy Disclosure Audit Message refers to ASTM 2147-01 which has been deprecated (for good or ill) for 2147-02.

The requirements of an accounting of disclosures are defined in [ASTM-2147](https://www.astm.org/e2147-01.html). A disclosure shall include the following, when the value is known:

If IHE is going to ignore the deprecation of this standard, then some mention as to why should be included.

3:5.7.4 SAML Security Token defines a SAML Security Token to be

"In this section we use the term “SAML Security Token” in a general sense to refer to either (a) a XUA SAML token being used in an ITI-40, or (b) any other SAML token used to secure an Interoperability transaction."

This includes IUA with the SAML option, which is a OAuth Security Token discussed in 3:5.7.5 OAuth Security Token.

I suggest to use the term "X-User Assertion" for section 3:5.7.4 and restricting the text XUA.

Note: According to WS-Trust specification which is referenced in XUA, the X-User Assertion returned to the X-Service User should be opaque and may be encrypted. Also just returning a link where the assertion can be retrieved by the X-Service Provider. Thus there is no principal difference between Oauth and SAML.

SAML profiling is focused on when a SAML token is used. There is no profiling of obtaining (access control decision). This is because XUA only profiled the use.

The profiles can provide more specific to that profile element short descriptions, definitions, and comments. please provide comments where specific profiled elements would be needed or useful.

Listing two implementations (omitting HAPI FHIR) one of which is only listed as "Grahame’s Server" is not a valuable section. If this is going to be useful, it would need information on at least the top 5 implementations. Better would remove this section as it's not giving significant guidance for the IHE use case.

Just two remarks:

Section 1:52.1.1.1 says: "The double recording enables forensic analysis to detect failures better". I'm not sure if that's true, since the server and client audit trails to my experience are sufficient. Also, failure detection is typically done shortly after the problem occurred based on the application logs (e.g., log4j).

In Europe and especially in Switzerland the ATNA Audit trails are more important
for legal purpose, e.g., to collect evidences to be able to verify if malicious actions have been performed in the past.

This IG covers only basic RESTful http. Not covered are FHIR Operations, or advanced http activities like Patch, conditional create, conditional update, etc? What others are needed, for them please provide an example transaction that can be used in a profiled example.

Throughout this section, the text consistently refers to the fact that this is not XUA when it would be far more helpful to provide mappings and descriptions on how to covert from XUA to BasicAudit beyond a simple mapping table. Explaining what could be done to preserve information that would be kept in an XUA ATNA record when many fields have no direct mappings would be helpful. Simply saying that FHIR AuditEvent is more corded than DICOM AuditMessage isn't useful information when one is trying to implement a wat forward.

Additional guidance should be given to assist those moving from DICOM AuditMessage to FHIR AuditEvent when dealing with SAML should be provided or at least acknowledged that it would require extensions or other structures.

Welcome comments about other patterns that would be needed and defined.

This section has the following note:

TODO: not clear what to say here or how to say it.

If this is a request for feedback, it should have more clarity as to what needs to be said.

Says "The Audit Record Repository shown is not specialized, it is the ATNA Audit Record Repository with support for ATNA ATX:FHIR Feed Option, and Retrieve Audit Message Option."

Isn't the Audit Record Repository specialized with the second part of the sentence?

Section Number
http://build.fhir.org/ig/IHE/ITI.BasicAudit/branches/main/volume-1.html#1524131-minimal-auditevent

Issue Describe your issue. Don't write a book, but do include enough to indicate what you see as a problem.

The minimal AuditEvent does not produce an audit log that is not sensitive, just reduced sensitivity.

Double negation is difficult to understand, i read it as:
The minimal AuditEvent does produce an audit log that is sensitive, just reduced sensitivity.

which sounds strange.

Proposed Change
clarify wording

Priority:

• Low: Typo or other minor classification that an editor can manage. Requires no group discussion.

should be retrieval

Is the oAuth AuditEvent patterns appropriate, especially the opaque one. With Opaque is the last 32 characters biggenough yet not too big?

3:5.7.5 OAuth Security Token focuses exclusively on IUA which makes some sense as it's the IHE standard. However, as SMART on FHIR is the predominant exchange method of FHIR, some guidance should be given for BasicAudit when SoF used for an IHE FHIR environment.

In addition, UDAP is also becoming more used (in Carequality and potentially elsewhere) and guidance would likely make BasicAudit more relevant for the future.

make document defined, but optional, and add data.

The R4 version of AuditEvent uses extensible binding often, this has limited the ways that the AuditEvent can be constrained. R5 has relaxed these to either example or preferred binding, so some further can be done in this IG once R5 is released.

The audit examples are brought in from MHD, PIXm, and PDQm; and “adjusted” to fit the RESTful pattern. This adjustment is not necessary, but follows with the proposal that these RESTful patterns are used as patterns in other Implementation Guides. Thus, these need to be evaluated as to if the adjustment is useful, or not. There should be no reason to update MHD, PIXm, or PDQm if there is not benefit, but there should also be no problem updating them. The adjustment here was more as an exercise in determining if the pattern concept could work, and not an excercise in forcing a change.
The adjustment was to the .type and .subtype, and one profile was changed from import/create to read.

who.identifier and what.identifer, rather than the .reference, are used because it is expected that as audit logging is happening these values are directly available, and the resource reference is not known. However it is possible that the resource reference is known, should we add a MS on who.reference and what.reference to encourage recording of these “when they are known”?

RESTful profiles start with a non-patient specific one, then a derived one with the patient slice. Is this the cleanest?

The table in this section has multiple blank lines with no mapping. Removing those and noting that elements not shown are not mapped would simplify the table and ease understanding.

put homeCommunityId in as an Agent. Given that the definition in XUA is standalone, and is independent of organization or user.