Conversation
malessi
requested review from
alex-dzeda,
aschey-forpeople,
mel1-G and
mjburling
as code owners
| shortDefinition = OpenAPIContentProvider.EOB_SERVICE_DATE_SHORT, | ||
| value = OpenAPIContentProvider.EOB_SERVICE_DATE_VALUE) | ||
| DateRangeParam serviceDate, | ||
| @OptionalParam(name = "includeTaxNumbers") |
There was a problem hiding this comment.
We need to leave the parameter alone, otherwise it will cause existing requests that supply this to fail.
There was a problem hiding this comment.
(Sorry, we discussed this privately, but think I forgot to mention this when we were refining it)
There was a problem hiding this comment.
Yep, I recall us talking about this. I'll re-add this parameter to all of the relevant resource providers but leave it unused.
There was a problem hiding this comment.
Done in 20672d0. Verified by:
xh --verify no --cert ... \
'https://localhost:6500/v2/fhir/ExplanationOfBenefit?patient=-88888888888888&includeTaxNumbers=false' \
'accept: application/fhir+json'and
xh --verify no --cert ... \
'https://localhost:6500/v1/fhir/ExplanationOfBenefit?patient=-88888888888888&includeTaxNumbers=false' \
'accept: application/fhir+json'| computeMDCKey(MDC_PREFIX, REQUEST_PREFIX, "clientSSL_DN"); | ||
|
|
||
| /** MDC key for the http request header include tax numbers. */ | ||
| public static final String HTTP_ACCESS_REQUEST_HEADER_TAX_NUMBERS = |
There was a problem hiding this comment.
Maybe we leave this? Just because we may want to remove it later and it would be good to track who is actually supplying this still.
There was a problem hiding this comment.
Yeah, I think that's probably a good idea. I'll revert this.
|
2 participants
JIRA Ticket:
BFD-4489
What Does This PR Do?
This PR completely removes
IncludeTaxNumbersfrom the resource provider and transformer layer of the BFD v1/v2 Server. Essentially, v1/v2 will now default to never including tax number information in any responses.Note that not all tax number related code has been removed from v1/v2, only the minimally necessary amount of logic has been removed such that tax number information is not included in responses and tests continue to pass.
What Should Reviewers Watch For?
If you're reviewing this PR, please check for these things in particular:
What Security Implications Does This PR Have?
Please indicate if this PR does any of the following:
Adds any new software dependencies
Modifies any security controls
Adds new transmission or storage of data
Any other changes that could possibly affect security?
I have considered the above security implications as it relates to this PR. (If one or more of the above apply, it cannot be merged without the ISSO or team security engineer's (
@sb-benohe) approval.)I have created tests to sufficiently ensure the reliability of my code, if applicable. If this is a modification to an existing piece of code, I have audited the associated tests to ensure everything works as expected.
Validation
Have you fully verified and tested these changes? Is the acceptance criteria met? Please provide reproducible testing instructions, code snippets, or screenshots as applicable.
_IncludeTaxNumbersandincludeTaxNumbersas a query parameter andIncludeTaxNumbersas a header, verifying that:openapi.yamldocument, verifying thatIncludeTaxNumbersis no longer documented