From b5ffb4068ac298c8ae8063cab805c159ad1a0d36 Mon Sep 17 00:00:00 2001 From: Luke deGruchy Date: Mon, 11 Mar 2024 17:27:15 -0400 Subject: [PATCH] First stab at a solution --- .../auth/AuthorizationInterceptor.java | 35 +++++++++++++++---- .../server/interceptor/auth/RuleBuilder.java | 9 ++++- .../server/interceptor/auth/RuleImplOp.java | 15 ++++++++ 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java index 1efa0e7e751..b421d19d961 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java @@ -159,6 +159,12 @@ public class AuthorizationInterceptor implements IRuleApplier { rules = buildRuleList(theRequestDetails); theRequestDetails.getUserData().put(myRequestRuleListKey, rules); } + // LUKETODO: rules do not contain any reference to PATCH + /* + 0 = {RuleImplOp@40767} "RuleImplOp[testers=,op=TRANSACTION,transactionAppliesToOp=ANY_OPERATION,appliesTo=,appliesToTypes=,classifierCompartmentName=,classifierCompartmentOwners=,classifierType=]" + 1 = {RuleImplOp@40768} "RuleImplOp[testers=,op=WRITE,transactionAppliesToOp=,appliesTo=TYPES,appliesToTypes=[Patient],classifierCompartmentName=,classifierCompartmentOwners=,classifierType=ANY_ID]" + */ + Set flags = getFlags(); ourLog.trace( @@ -168,10 +174,26 @@ public class AuthorizationInterceptor implements IRuleApplier { getResourceTypeOrEmpty(theInputResource), getResourceTypeOrEmpty(theOutputResource)); + Verdict verdict = null; - for (IAuthRule nextRule : rules) { - ourLog.trace("Rule being applied - {}", nextRule); - verdict = nextRule.applyRule( + + // LUKETODO: try to just check for FHIR_PATCH if this is a FHIR_PATCH and if it's not there, then return a deny verdict + if (theOperation == RestOperationTypeEnum.PATCH) { +// if (rules.stream() +// .filter(RuleImplOp.class::isInstance) +// .map(RuleImplOp.class::cast) +// .noneMatch(rule -> rule.getOp() == RuleOpEnum.PATCH)) { + if (rules.stream() + .noneMatch(RuleImplPatch.class::isInstance)) { + // LUKETODO: this results in a 403 but is that what we want? + verdict = new Verdict(PolicyEnum.DENY, null); + } + } + + if (verdict == null) { + for (IAuthRule nextRule : rules) { + ourLog.trace("Rule being applied - {}", nextRule); + verdict = nextRule.applyRule( theOperation, theRequestDetails, theInputResource, @@ -180,9 +202,10 @@ public class AuthorizationInterceptor implements IRuleApplier { this, flags, thePointcut); - if (verdict != null) { - ourLog.trace("Rule {} returned decision {}", nextRule, verdict.getDecision()); - break; + if (verdict != null) { + ourLog.trace("Rule {} returned decision {}", nextRule, verdict.getDecision()); + break; + } } } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java index 9316cd89d35..d7562cdd1dc 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java @@ -834,10 +834,17 @@ public class RuleBuilder implements IAuthRuleBuilder { @Override public IAuthRuleFinished allRequests() { + // LUKETODO: should we be building this simple ruleImplOp or should be we basing this off RuleImplOp? BaseRule rule = new RuleImplPatch(myRuleName).setAllRequests(true).setMode(myRuleMode); + // LUKETODO: HTTP 500 Server Error: HAPI-0335: Unable to apply security to event of type PATCH + RuleImplOp ruleImplOp = new RuleImplOp(myRuleName); + ruleImplOp.setMode(myRuleMode); + ruleImplOp.setOp(RuleOpEnum.PATCH); + ruleImplOp.setTransactionAppliesToOp(TransactionAppliesToEnum.ANY_OPERATION); +// myRules.add(ruleImplOp); myRules.add(rule); - return new RuleBuilderFinished(rule); + return new RuleBuilderFinished(ruleImplOp); } } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java index e6b08be356e..4f92065bb1a 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java @@ -240,6 +240,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { } break; case PATCH: + // LUKETODO: should we just short-circuit here and let the PATCH case decide this? target.resource = null; if (theInputResourceId != null) { target.resourceIds = Collections.singletonList(theInputResourceId); @@ -348,6 +349,16 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { theRuleApplier); } return null; +// case PATCH: +// // LUKETODO: ? +// // LUKETODO: do we need applies to types to correspond to +// target.resource = null; +// if (theInputResourceId != null) { +// target.resourceIds = Collections.singletonList(theInputResourceId); +// } else { +// return null; +// } +// break; default: // Should not happen throw new IllegalStateException( @@ -959,6 +970,10 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { return false; } + public RuleOpEnum getOp() { + return myOp; + } + public void setAppliesTo(AppliesTypeEnum theAppliesTo) { myAppliesTo = theAppliesTo; }