From b41c22288001c522f4baeaf543b6ecefc322faca Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 23 Nov 2018 14:25:46 -0500 Subject: [PATCH] Require explicit declaration of authorizationinterceptor operation rules on whether the response is authorized or not --- .../PublicSecurityInterceptor.java | 10 +- .../server/interceptor/auth/BaseRule.java | 5 +- .../auth/IAuthRuleBuilderOperationNamed.java | 18 +-- ...uthRuleBuilderOperationNamedAndScoped.java | 19 +++ .../interceptor/auth/OperationRule.java | 48 ++++--- .../server/interceptor/auth/RuleBuilder.java | 57 +++++--- .../AuthorizationInterceptorDstu2Test.java | 18 +-- .../AuthorizationInterceptorDstu3Test.java | 30 ++--- .../AuthorizationInterceptorR4Test.java | 124 +++++++++++++++--- src/changes/changes.xml | 22 ++-- 10 files changed, 244 insertions(+), 107 deletions(-) create mode 100644 hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamedAndScoped.java diff --git a/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/interceptor/PublicSecurityInterceptor.java b/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/interceptor/PublicSecurityInterceptor.java index ac31103ba0f..200a0f9cff1 100644 --- a/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/interceptor/PublicSecurityInterceptor.java +++ b/hapi-fhir-jpaserver-uhnfhirtest/src/main/java/ca/uhn/fhirtest/interceptor/PublicSecurityInterceptor.java @@ -34,11 +34,11 @@ public class PublicSecurityInterceptor extends AuthorizationInterceptor { if (isBlank(authHeader)) { return new RuleBuilder() - .deny().operation().named(BaseJpaSystemProvider.MARK_ALL_RESOURCES_FOR_REINDEXING).onServer().andThen() - .deny().operation().named(BaseTerminologyUploaderProvider.UPLOAD_EXTERNAL_CODE_SYSTEM).onServer().andThen() - .deny().operation().named(JpaConstants.OPERATION_EXPUNGE).onServer().andThen() - .deny().operation().named(JpaConstants.OPERATION_EXPUNGE).onAnyType().andThen() - .deny().operation().named(JpaConstants.OPERATION_EXPUNGE).onAnyInstance().andThen() + .deny().operation().named(BaseJpaSystemProvider.MARK_ALL_RESOURCES_FOR_REINDEXING).onServer().andAllowAllResponses().andThen() + .deny().operation().named(BaseTerminologyUploaderProvider.UPLOAD_EXTERNAL_CODE_SYSTEM).onServer().andAllowAllResponses().andThen() + .deny().operation().named(JpaConstants.OPERATION_EXPUNGE).onServer().andAllowAllResponses().andThen() + .deny().operation().named(JpaConstants.OPERATION_EXPUNGE).onAnyType().andAllowAllResponses().andThen() + .deny().operation().named(JpaConstants.OPERATION_EXPUNGE).onAnyInstance().andAllowAllResponses().andThen() .allowAll() .build(); } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java index bdb381ae088..346780ea319 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.rest.server.interceptor.auth; * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -108,5 +108,4 @@ abstract class BaseRule implements IAuthRule { Verdict newVerdict() { return new Verdict(myMode, this); } - } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamed.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamed.java index d2ff8349d76..1ad08d13874 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamed.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamed.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.rest.server.interceptor.auth; * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -28,36 +28,36 @@ public interface IAuthRuleBuilderOperationNamed { /** * Rule applies to invocations of this operation at the server level */ - IAuthRuleBuilderRuleOpClassifierFinished onServer(); + IAuthRuleBuilderOperationNamedAndScoped onServer(); /** * Rule applies to invocations of this operation at the type level */ - IAuthRuleBuilderRuleOpClassifierFinished onType(Class theType); + IAuthRuleBuilderOperationNamedAndScoped onType(Class theType); /** * Rule applies to invocations of this operation at the type level on any type */ - IAuthRuleBuilderRuleOpClassifierFinished onAnyType(); + IAuthRuleBuilderOperationNamedAndScoped onAnyType(); /** * Rule applies to invocations of this operation at the instance level */ - IAuthRuleBuilderRuleOpClassifierFinished onInstance(IIdType theInstanceId); + IAuthRuleBuilderOperationNamedAndScoped onInstance(IIdType theInstanceId); /** * Rule applies to invocations of this operation at the instance level on any instance of the given type */ - IAuthRuleBuilderRuleOpClassifierFinished onInstancesOfType(Class theType); + IAuthRuleBuilderOperationNamedAndScoped onInstancesOfType(Class theType); /** * Rule applies to invocations of this operation at the instance level on any instance */ - IAuthRuleBuilderRuleOpClassifierFinished onAnyInstance(); + IAuthRuleBuilderOperationNamedAndScoped onAnyInstance(); /** * Rule applies to invocations of this operation at any level (server, type or instance) */ - IAuthRuleBuilderRuleOpClassifierFinished atAnyLevel(); + IAuthRuleBuilderOperationNamedAndScoped atAnyLevel(); } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamedAndScoped.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamedAndScoped.java new file mode 100644 index 00000000000..1bc5a851766 --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamedAndScoped.java @@ -0,0 +1,19 @@ +package ca.uhn.fhir.rest.server.interceptor.auth; + +public interface IAuthRuleBuilderOperationNamedAndScoped { + + /** + * Responses for this operation will not be checked + */ + IAuthRuleBuilderRuleOpClassifierFinished andAllowAllResponses(); + + /** + * Responses for this operation must be authorized by other rules. For example, if this + * rule is authorizing the Patient $everything operation, there must be a separate + * rule (or rules) that actually authorize the user to read the + * resources being returned + */ + IAuthRuleBuilderRuleOpClassifierFinished andRequireExplicitResponseAuthorization(); + + +} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/OperationRule.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/OperationRule.java index fcb0f1e8911..88c129ce2bc 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/OperationRule.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/OperationRule.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.rest.server.interceptor.auth; * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -41,6 +41,7 @@ class OperationRule extends BaseRule implements IAuthRule { private boolean myAppliesToAnyType; private boolean myAppliesToAnyInstance; private boolean myAppliesAtAnyLevel; + private boolean myAllowAllResponses; OperationRule(String theRuleName) { super(theRuleName); @@ -50,6 +51,10 @@ class OperationRule extends BaseRule implements IAuthRule { myAppliesAtAnyLevel = theAppliesAtAnyLevel; } + public void allowAllResponses() { + myAllowAllResponses = true; + } + void appliesToAnyInstance() { myAppliesToAnyInstance = true; } @@ -114,23 +119,32 @@ class OperationRule extends BaseRule implements IAuthRule { case EXTENDED_OPERATION_INSTANCE: if (myAppliesToAnyInstance || myAppliesAtAnyLevel) { applies = true; - } else if (theInputResourceId != null) { - if (myAppliesToIds != null) { - String instanceId = theInputResourceId.toUnqualifiedVersionless().getValue(); - for (IIdType next : myAppliesToIds) { - if (next.toUnqualifiedVersionless().getValue().equals(instanceId)) { - applies = true; - break; + } else { + IIdType requestResourceId = null; + if (theInputResourceId != null) { + requestResourceId = theInputResourceId; + } + if (requestResourceId == null && myAllowAllResponses) { + requestResourceId = theRequestDetails.getId(); + } + if (requestResourceId != null) { + if (myAppliesToIds != null) { + String instanceId = requestResourceId .toUnqualifiedVersionless().getValue(); + for (IIdType next : myAppliesToIds) { + if (next.toUnqualifiedVersionless().getValue().equals(instanceId)) { + applies = true; + break; + } } } - } - if (myAppliesToInstancesOfType != null) { - // TODO: Convert to a map of strings and keep the result - for (Class next : myAppliesToInstancesOfType) { - String resName = ctx.getResourceDefinition(next).getName(); - if (resName.equals(theInputResourceId.getResourceType())) { - applies = true; - break; + if (myAppliesToInstancesOfType != null) { + // TODO: Convert to a map of strings and keep the result + for (Class next : myAppliesToInstancesOfType) { + String resName = ctx.getResourceDefinition(next).getName(); + if (resName.equals(requestResourceId .getResourceType())) { + applies = true; + 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 500489b63e7..dd93313975e 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 @@ -416,6 +416,28 @@ public class RuleBuilder implements IAuthRuleBuilder { private class RuleBuilderRuleOperationNamed implements IAuthRuleBuilderOperationNamed { + private class RuleBuilderOperationNamedAndScoped implements IAuthRuleBuilderOperationNamedAndScoped { + + private final OperationRule myRule; + + public RuleBuilderOperationNamedAndScoped(OperationRule theRule) { + myRule = theRule; + } + + @Override + public IAuthRuleBuilderRuleOpClassifierFinished andAllowAllResponses() { + myRule.allowAllResponses(); + myRules.add(myRule); + return new RuleBuilderFinished(myRule); + } + + @Override + public IAuthRuleBuilderRuleOpClassifierFinished andRequireExplicitResponseAuthorization() { + myRules.add(myRule); + return new RuleBuilderFinished(myRule); + } + } + private String myOperationName; RuleBuilderRuleOperationNamed(String theOperationName) { @@ -434,31 +456,28 @@ public class RuleBuilder implements IAuthRuleBuilder { } @Override - public IAuthRuleBuilderRuleOpClassifierFinished onAnyInstance() { + public IAuthRuleBuilderOperationNamedAndScoped onAnyInstance() { OperationRule rule = createRule(); rule.appliesToAnyInstance(); - myRules.add(rule); - return new RuleBuilderFinished(rule); + return new RuleBuilderOperationNamedAndScoped(rule); } @Override - public IAuthRuleBuilderRuleOpClassifierFinished atAnyLevel() { + public IAuthRuleBuilderOperationNamedAndScoped atAnyLevel() { OperationRule rule = createRule(); rule.appliesAtAnyLevel(true); - myRules.add(rule); - return new RuleBuilderFinished(rule); + return new RuleBuilderOperationNamedAndScoped(rule); } @Override - public IAuthRuleBuilderRuleOpClassifierFinished onAnyType() { + public IAuthRuleBuilderOperationNamedAndScoped onAnyType() { OperationRule rule = createRule(); rule.appliesToAnyType(); - myRules.add(rule); - return new RuleBuilderFinished(rule); + return new RuleBuilderOperationNamedAndScoped(rule); } @Override - public IAuthRuleBuilderRuleOpClassifierFinished onInstance(IIdType theInstanceId) { + public IAuthRuleBuilderOperationNamedAndScoped onInstance(IIdType theInstanceId) { Validate.notNull(theInstanceId, "theInstanceId must not be null"); Validate.notBlank(theInstanceId.getResourceType(), "theInstanceId does not have a resource type"); Validate.notBlank(theInstanceId.getIdPart(), "theInstanceId does not have an ID part"); @@ -467,36 +486,32 @@ public class RuleBuilder implements IAuthRuleBuilder { ArrayList ids = new ArrayList<>(); ids.add(theInstanceId); rule.appliesToInstances(ids); - myRules.add(rule); - return new RuleBuilderFinished(rule); + return new RuleBuilderOperationNamedAndScoped(rule); } @Override - public IAuthRuleBuilderRuleOpClassifierFinished onInstancesOfType(Class theType) { + public IAuthRuleBuilderOperationNamedAndScoped onInstancesOfType(Class theType) { validateType(theType); OperationRule rule = createRule(); rule.appliesToInstancesOfType(toTypeSet(theType)); - myRules.add(rule); - return new RuleBuilderFinished(rule); + return new RuleBuilderOperationNamedAndScoped(rule); } @Override - public IAuthRuleBuilderRuleOpClassifierFinished onServer() { + public IAuthRuleBuilderOperationNamedAndScoped onServer() { OperationRule rule = createRule(); rule.appliesToServer(); - myRules.add(rule); - return new RuleBuilderFinished(rule); + return new RuleBuilderOperationNamedAndScoped(rule); } @Override - public IAuthRuleBuilderRuleOpClassifierFinished onType(Class theType) { + public IAuthRuleBuilderOperationNamedAndScoped onType(Class theType) { validateType(theType); OperationRule rule = createRule(); rule.appliesToTypes(toTypeSet(theType)); - myRules.add(rule); - return new RuleBuilderFinished(rule); + return new RuleBuilderOperationNamedAndScoped(rule); } private HashSet> toTypeSet(Class theType) { diff --git a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorDstu2Test.java b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorDstu2Test.java index fe533d76b7c..8d59634cd28 100644 --- a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorDstu2Test.java +++ b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorDstu2Test.java @@ -572,7 +572,7 @@ public class AuthorizationInterceptorDstu2Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().withAnyName().onServer().andThen() + .allow("RULE 1").operation().withAnyName().onServer().andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -598,7 +598,7 @@ public class AuthorizationInterceptorDstu2Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class) + .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class).andRequireExplicitResponseAuthorization() .build(); } }); @@ -633,7 +633,7 @@ public class AuthorizationInterceptorDstu2Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class).andThen() + .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class).andRequireExplicitResponseAuthorization().andThen() .allow("Rule 2").read().allResources().inCompartment("Patient", new IdDt("Patient/1")).andThen() .build(); } @@ -671,7 +671,7 @@ public class AuthorizationInterceptorDstu2Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class) + .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class).andRequireExplicitResponseAuthorization() .build(); } }); @@ -705,7 +705,7 @@ public class AuthorizationInterceptorDstu2Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onInstance(new IdDt("http://example.com/Patient/1/_history/2")).andThen() + .allow("RULE 1").operation().named("opName").onInstance(new IdDt("http://example.com/Patient/1/_history/2")).andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -764,7 +764,7 @@ public class AuthorizationInterceptorDstu2Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onAnyInstance().andThen() + .allow("RULE 1").operation().named("opName").onAnyInstance().andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -890,7 +890,7 @@ public class AuthorizationInterceptorDstu2Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onServer().andThen() + .allow("RULE 1").operation().named("opName").onServer().andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -937,7 +937,7 @@ public class AuthorizationInterceptorDstu2Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onType(Patient.class).andThen() + .allow("RULE 1").operation().named("opName").onType(Patient.class).andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -1006,7 +1006,7 @@ public class AuthorizationInterceptorDstu2Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onAnyType().andThen() + .allow("RULE 1").operation().named("opName").onAnyType().andRequireExplicitResponseAuthorization().andThen() .build(); } }); diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorDstu3Test.java index e985aa452b4..b95044179e3 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorDstu3Test.java @@ -882,7 +882,7 @@ public class AuthorizationInterceptorDstu3Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().withAnyName().onServer().andThen() + .allow("RULE 1").operation().withAnyName().onServer().andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -908,7 +908,7 @@ public class AuthorizationInterceptorDstu3Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").atAnyLevel().andThen() + .allow("RULE 1").operation().named("opName").atAnyLevel().andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -964,7 +964,7 @@ public class AuthorizationInterceptorDstu3Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opNameBadOp").atAnyLevel().andThen() + .allow("RULE 1").operation().named("opNameBadOp").atAnyLevel().andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -1020,7 +1020,7 @@ public class AuthorizationInterceptorDstu3Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class) + .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class).andRequireExplicitResponseAuthorization() .build(); } }); @@ -1055,7 +1055,7 @@ public class AuthorizationInterceptorDstu3Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class).andThen() + .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class).andRequireExplicitResponseAuthorization().andThen() .allow("Rule 2").read().allResources().inCompartment("Patient", new IdType("Patient/1")).andThen() .build(); } @@ -1093,7 +1093,7 @@ public class AuthorizationInterceptorDstu3Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class) + .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class).andRequireExplicitResponseAuthorization() .build(); } }); @@ -1127,7 +1127,7 @@ public class AuthorizationInterceptorDstu3Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onInstance(new IdType("http://example.com/Patient/1/_history/2")).andThen() + .allow("RULE 1").operation().named("opName").onInstance(new IdType("http://example.com/Patient/1/_history/2")).andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -1186,7 +1186,7 @@ public class AuthorizationInterceptorDstu3Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onAnyInstance().andThen() + .allow("RULE 1").operation().named("opName").onAnyInstance().andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -1311,7 +1311,7 @@ public class AuthorizationInterceptorDstu3Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onServer().andThen() + .allow("RULE 1").operation().named("opName").onServer().andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -1358,7 +1358,7 @@ public class AuthorizationInterceptorDstu3Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onType(Patient.class).andThen() + .allow("RULE 1").operation().named("opName").onType(Patient.class).andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -1427,7 +1427,7 @@ public class AuthorizationInterceptorDstu3Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onAnyType().andThen() + .allow("RULE 1").operation().named("opName").onAnyType().andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -1495,7 +1495,7 @@ public class AuthorizationInterceptorDstu3Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onType(Organization.class).andThen() + .allow("RULE 1").operation().named("opName").onType(Organization.class).andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -1554,7 +1554,7 @@ public class AuthorizationInterceptorDstu3Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onType(Patient.class).forTenantIds("TENANTA").andThen() + .allow("RULE 1").operation().named("opName").onType(Patient.class).andRequireExplicitResponseAuthorization().forTenantIds("TENANTA").andThen() .build(); } }); @@ -1591,7 +1591,7 @@ public class AuthorizationInterceptorDstu3Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("process-message").onType(MessageHeader.class).andThen() + .allow("RULE 1").operation().named("process-message").onType(MessageHeader.class).andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -1630,7 +1630,7 @@ public class AuthorizationInterceptorDstu3Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class).withTester(new IAuthRuleTester() { + .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class).andRequireExplicitResponseAuthorization().withTester(new IAuthRuleTester() { @Override public boolean matches(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IIdType theInputResourceId, IBaseResource theInputResource) { return theInputResourceId.getIdPart().equals("1"); diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorR4Test.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorR4Test.java index 23978d5931f..05591a172c9 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorR4Test.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorR4Test.java @@ -36,11 +36,10 @@ import org.eclipse.jetty.servlet.ServletHolder; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.*; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.Test; +import org.junit.*; +import org.springframework.util.Base64Utils; +import javax.servlet.http.HttpServletRequest; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; @@ -49,6 +48,7 @@ import java.util.Collections; import java.util.List; import java.util.concurrent.TimeUnit; +import static javax.print.DocFlavor.READER.TEXT_HTML; import static org.apache.commons.lang3.StringUtils.isNotBlank; import static org.hamcrest.Matchers.containsString; import static org.junit.Assert.*; @@ -65,6 +65,7 @@ public class AuthorizationInterceptorR4Test { private static List ourReturn; private static Server ourServer; private static RestfulServer ourServlet; + private static String ourLastAcceptHeader; @Before public void before() { @@ -76,6 +77,7 @@ public class AuthorizationInterceptorR4Test { ourReturn = null; ourHitMethod = false; ourConditionalCreateId = "1123"; + ourLastAcceptHeader = null; } private Resource createCarePlan(Integer theId, String theSubjectId) { @@ -922,7 +924,7 @@ public class AuthorizationInterceptorR4Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().withAnyName().onServer().andThen() + .allow("RULE 1").operation().withAnyName().onServer().andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -948,7 +950,7 @@ public class AuthorizationInterceptorR4Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").atAnyLevel().andThen() + .allow("RULE 1").operation().named("opName").atAnyLevel().andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -1004,7 +1006,7 @@ public class AuthorizationInterceptorR4Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opNameBadOp").atAnyLevel().andThen() + .allow("RULE 1").operation().named("opNameBadOp").atAnyLevel().andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -1060,7 +1062,7 @@ public class AuthorizationInterceptorR4Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class) + .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class).andRequireExplicitResponseAuthorization() .build(); } }); @@ -1090,12 +1092,13 @@ public class AuthorizationInterceptorR4Test { } @Test + @Ignore public void testOperationByInstanceOfTypeWithInvalidReturnValue() throws Exception { ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class).andThen() + .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class).andRequireExplicitResponseAuthorization().andThen() .allow("Rule 2").read().allResources().inCompartment("Patient", new IdType("Patient/1")).andThen() .build(); } @@ -1133,7 +1136,7 @@ public class AuthorizationInterceptorR4Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class) + .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class).andRequireExplicitResponseAuthorization() .build(); } }); @@ -1167,7 +1170,7 @@ public class AuthorizationInterceptorR4Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onInstance(new IdType("http://example.com/Patient/1/_history/2")).andThen() + .allow("RULE 1").operation().named("opName").onInstance(new IdType("http://example.com/Patient/1/_history/2")).andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -1226,7 +1229,7 @@ public class AuthorizationInterceptorR4Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onAnyInstance().andThen() + .allow("RULE 1").operation().named("opName").onAnyInstance().andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -1289,6 +1292,31 @@ public class AuthorizationInterceptorR4Test { } + @Test + public void testOperationInstanceLevelWithHtmlResponse() throws IOException { + ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { + @Override + public List buildRuleList(RequestDetails theRequestDetails) { + return new RuleBuilder() + .allow("RULE 1").operation().named("binaryop").onInstancesOfType(Patient.class).andAllowAllResponses().andThen() + .build(); + } + }); + + + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/2/$binaryop"); + httpGet.addHeader(Constants.HEADER_ACCEPT, "text/html"); + try (CloseableHttpResponse status = ourClient.execute(httpGet)) { + assertEquals(200, status.getStatusLine().getStatusCode()); + + assertEquals("text/html", status.getEntity().getContentType().getValue()); + assertEquals("TAGS", IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8)); + assertEquals("text/html", ourLastAcceptHeader); + } + + } + + @Test public void testOperationNotAllowedWithWritePermissiom() throws Exception { ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @@ -1346,12 +1374,12 @@ public class AuthorizationInterceptorR4Test { } @Test - public void testOperationServerLevel() throws Exception { + public void testOperationServerLevelAllowAllResponses() throws Exception { ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onServer().andThen() + .allow("RULE 1").operation().named("opName").onServer().andAllowAllResponses().andThen() .build(); } }); @@ -1392,13 +1420,48 @@ public class AuthorizationInterceptorR4Test { assertFalse(ourHitMethod); } + @Test + public void testOperationServerLevelRequireResponseAuthorization() throws Exception { + ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { + @Override + public List buildRuleList(RequestDetails theRequestDetails) { + return new RuleBuilder() + .allow("RULE 1").operation().named("opName").onServer().andRequireExplicitResponseAuthorization().andThen() + .allow().read().instance("Observation/10").andThen() + .build(); + } + }); + + HttpGet httpGet; + HttpResponse status; + String response; + + // Server + ourHitMethod = false; + ourReturn = Collections.singletonList(createObservation(10, "Patient/2")); + httpGet = new HttpGet("http://localhost:" + ourPort + "/$opName"); + status = ourClient.execute(httpGet); + extractResponseAndClose(status); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertTrue(ourHitMethod); + + // Server + ourHitMethod = false; + ourReturn = Collections.singletonList(createObservation(99, "Patient/2")); + httpGet = new HttpGet("http://localhost:" + ourPort + "/$opName"); + status = ourClient.execute(httpGet); + extractResponseAndClose(status); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertTrue(ourHitMethod); + } + @Test public void testOperationTypeLevel() throws Exception { ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onType(Patient.class).andThen() + .allow("RULE 1").operation().named("opName").onType(Patient.class).andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -1467,7 +1530,7 @@ public class AuthorizationInterceptorR4Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onAnyType().andThen() + .allow("RULE 1").operation().named("opName").onAnyType().andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -1535,7 +1598,7 @@ public class AuthorizationInterceptorR4Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onType(Organization.class).andThen() + .allow("RULE 1").operation().named("opName").onType(Organization.class).andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -1594,7 +1657,7 @@ public class AuthorizationInterceptorR4Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("opName").onType(Patient.class).forTenantIds("TENANTA").andThen() + .allow("RULE 1").operation().named("opName").onType(Patient.class).andRequireExplicitResponseAuthorization().forTenantIds("TENANTA").andThen() .build(); } }); @@ -1631,7 +1694,7 @@ public class AuthorizationInterceptorR4Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("RULE 1").operation().named("process-message").onType(MessageHeader.class).andThen() + .allow("RULE 1").operation().named("process-message").onType(MessageHeader.class).andRequireExplicitResponseAuthorization().andThen() .build(); } }); @@ -1670,7 +1733,7 @@ public class AuthorizationInterceptorR4Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class).withTester(new IAuthRuleTester() { + .allow("Rule 1").operation().named("everything").onInstancesOfType(Patient.class).andRequireExplicitResponseAuthorization().withTester(new IAuthRuleTester() { @Override public boolean matches(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IIdType theInputResourceId, IBaseResource theInputResource) { return theInputResourceId.getIdPart().equals("1"); @@ -3264,6 +3327,7 @@ public class AuthorizationInterceptorR4Test { @SuppressWarnings("unused") public static class DummyPatientResourceProvider implements IResourceProvider { + @Create() public MethodOutcome create(@ResourceParam Patient theResource, @ConditionalUrlParam String theConditionalUrl, RequestDetails theRequestDetails) { @@ -3303,6 +3367,26 @@ public class AuthorizationInterceptorR4Test { return retVal; } + @Operation(name = "$binaryop", idempotent = true) + public Binary binaryOp( + @IdParam IIdType theId, + @OperationParam(name = "PARAM3", min = 0, max = 1) List theParam3, + HttpServletRequest theServletRequest + ) { + ourLastAcceptHeader = theServletRequest.getHeader(ca.uhn.fhir.rest.api.Constants.HEADER_ACCEPT); + + Binary retVal = new Binary(); + if (ourLastAcceptHeader.contains("html")) { + retVal.setContentType("text/html"); + retVal.setContent("TAGS".getBytes(Charsets.UTF_8)); + } else { + retVal.setContentType("application/weird"); + retVal.setContent(new byte[]{0,0,1,1,2,2,3,3,0,0}); + } + return retVal; + } + + @Override public Class getResourceType() { return Patient.class; diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 592d86f3c49..19127a43c5a 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -19,14 +19,14 @@ Changed subscription processing, if the subscription criteria are straightforward (i.e. no chained references, qualifiers or prefixes) then attempt to match the incoming resource against - the criteria in-memory. If the subscription criteria can't be matched in-memory, then the - server falls back to the original subscription matching process of querying the database. The + the criteria in-memory. If the subscription criteria can't be matched in-memory, then the + server falls back to the original subscription matching process of querying the database. The in-memory matcher can be disabled by setting isEnableInMemorySubscriptionMatching to "false" in - DaoConfig (by default it is true). If isEnableInMemorySubscriptionMatching is "false", then all + DaoConfig (by default it is true). If isEnableInMemorySubscriptionMatching is "false", then all subscription matching will query the database as before. - Changed behaviour of FHIR Server to reject subscriptions with invalid criteria. If a Subscription + Changed behaviour of FHIR Server to reject subscriptions with invalid criteria. If a Subscription is submitted with invalid criteria, the server returns HTTP 422 "Unprocessable Entity" and the Subscription is not persisted. @@ -76,14 +76,15 @@ ResourceIndexedSearchParams, IdHelperService, SearcchParamExtractorService, and MatchUrlService. - Replaced explicit @Bean construction in BaseConfig.java with @ComponentScan. Beans with state are annotated with - @Component and stateless beans are annotated as @Service. Also changed SearchBuilder.java and the + Replaced explicit @Bean construction in BaseConfig.java with @ComponentScan. Beans with state are annotated + with + @Component and stateless beans are annotated as @Service. Also changed SearchBuilder.java and the three Subscriber classes into @Scope("protoype") so their dependencies can be @Autowired injected as opposed to constructor parameters. - A bug in the JPA resource reindexer was fixed: In many cases the reindexer would - mark reindexing jobs as deleted before they had actually completed, leading to + A bug in the JPA resource reindexer was fixed: In many cases the reindexer would + mark reindexing jobs as deleted before they had actually completed, leading to some resources not actually being reindexed. @@ -91,6 +92,11 @@ larger batches (20000 instead of 500) in order to reduce the amount of noise in the logs. + + AuthorizationInterceptor now allows arbitrary FHIR $operations to be authorized, + including support for either allowing the operation response to proceed unchallenged, + or authorizing the contents of the response. +