issue #3495 - more selectively scope searches to patient compartment #3498
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Specifically, avoid adding the implicit compartment parameter(s) when
the interaction is covered by one or more tokens with the 'user/' or
'system/' prefix ("context type")
Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
punktilious
reviewed
Mar 23, 2022
fhir-smart/src/main/java/com/ibm/fhir/smart/AuthzPolicyEnforcementPersistenceInterceptor.java
Show resolved
Hide resolved
punktilious
reviewed
Mar 23, 2022
fhir-smart/src/main/java/com/ibm/fhir/smart/AuthzPolicyEnforcementPersistenceInterceptor.java
Show resolved
Hide resolved
punktilious
reviewed
Mar 23, 2022
| .filter(s -> s.getResourceType() == ResourceType.Value.RESOURCE || s.getResourceType().value().equals(resourceType)) | ||
| .filter(s -> hasPermission(s.getPermission(), requiredPermission)) | ||
| .findAny(); | ||
|
|
||
| if (resource == null) { | ||
| // For `patient` scopes, if no resource was retrieved, which happens with history/search |
There was a problem hiding this comment.
what about deleted resources? The resource value will be null for those too
There was a problem hiding this comment.
we don't call this from afterDelete, but we do call it from afterHistory and so I agree this is worth testing...I'll give it a try and maybe add a test
There was a problem hiding this comment.
this wasn't really from this PR, but instead from #3423
however, i confirmed that, with those changes, we now reject a history interaction if any one of the resource versions in the history was a delete...i'll see if i can address that in this PR. if not, i'll open a separate issue for it
There was a problem hiding this comment.
I made some changes to try addressing this one. please have another look
punktilious
reviewed
Mar 23, 2022
tbieste
approved these changes
Mar 23, 2022
Also addressed review comments by adding class-level javadoc Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre
force-pushed
the
issue-3495
branch
2 times, most recently
from
dbbb102 to
7ad8f13
Compare
This fixes an issue that we introduced with #3423 where we reject history interactions that include one or more DELETE entries in their response bundle (because we cannot check them for patient compartment membership since they contain no resource content). However, now that we reject history interactions that involve patient compartment resource types (unless the approving scope is of type 'user' or 'system'), it should always be safe to ignore the delete entries in the afterHistory call. Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Specifically, avoid adding the implicit compartment parameter(s) when
the interaction is allowed by one or more scopes with the 'user/' or
'system/' prefix ("context type")
Signed-off-by: Lee Surprenant lmsurpre@us.ibm.com