diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java index dd26ab8dc06..5ac0f85e3f3 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java @@ -301,18 +301,6 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer return true; } - private List toListOfResourcesAndExcludeContainer(IBaseResource theResponseObject, FhirContext fhirContext) { - List resources; - resources = fhirContext.newTerser().getAllPopulatedChildElementsOfType(theResponseObject, IBaseResource.class); - - // Exclude the container - if (resources.size() > 0 && resources.get(0) == theResponseObject) { - resources = resources.subList(1, resources.size()); - } - - return resources; - } - @CoverageIgnore @Override public boolean outgoingResponse(RequestDetails theRequestDetails, TagList theResponseObject) { @@ -350,6 +338,18 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer myDefaultPolicy = theDefaultPolicy; } + private List toListOfResourcesAndExcludeContainer(IBaseResource theResponseObject, FhirContext fhirContext) { + List resources; + resources = fhirContext.newTerser().getAllPopulatedChildElementsOfType(theResponseObject, IBaseResource.class); + + // Exclude the container + if (resources.size() > 0 && resources.get(0) == theResponseObject) { + resources = resources.subList(1, resources.size()); + } + + return resources; + } + // private List toListOfResources(FhirContext fhirContext, IBaseBundle responseBundle) { // List retVal = BundleUtil.toListOfResources(fhirContext, responseBundle); // for (int i = 0; i < retVal.size(); i++) { @@ -368,10 +368,10 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer } private enum OperationExamineDirection { + BOTH, IN, - NONE, + NONE, OUT, - BOTH, } public static class Verdict { diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamed.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamed.java index 4d1e4d07e95..cb84a52ba06 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamed.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamed.java @@ -35,6 +35,11 @@ public interface IAuthRuleBuilderOperationNamed { */ IAuthRuleBuilderRuleOpClassifierFinished onType(Class theType); + /** + * Rule applies to invocations of this operation at the type level on any type + */ + IAuthRuleFinished onAnyType(); + /** * Rule applies to invocations of this operation at the instance level */ @@ -45,4 +50,9 @@ public interface IAuthRuleBuilderOperationNamed { */ IAuthRuleBuilderRuleOpClassifierFinished onInstancesOfType(Class theType); + /** + * Rule applies to invocations of this operation at the instance level on any instance + */ + IAuthRuleFinished onAnyInstance(); + } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/OperationRule.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/OperationRule.java index 71ec0c35425..40ee7b604a2 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/OperationRule.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/OperationRule.java @@ -42,6 +42,8 @@ class OperationRule extends BaseRule implements IAuthRule { private HashSet> myAppliesToTypes; private List myAppliesToIds; private HashSet> myAppliesToInstancesOfType; + private boolean myAppliesToAnyType; + private boolean myAppliesToAnyInstance; /** * Must include the leading $ @@ -66,7 +68,9 @@ class OperationRule extends BaseRule implements IAuthRule { } break; case EXTENDED_OPERATION_TYPE: - if (myAppliesToTypes != null) { + if (myAppliesToAnyType) { + applies = true; + } else if (myAppliesToTypes != null) { // TODO: Convert to a map of strings and keep the result for (Class next : myAppliesToTypes) { String resName = ctx.getResourceDefinition(next).getName(); @@ -78,7 +82,9 @@ class OperationRule extends BaseRule implements IAuthRule { } break; case EXTENDED_OPERATION_INSTANCE: - if (theInputResourceId != null) { + if (myAppliesToAnyInstance) { + applies = true; + } else if (theInputResourceId != null) { if (myAppliesToIds != null) { String instanceId = theInputResourceId.toUnqualifiedVersionless().getValue(); for (IIdType next : myAppliesToIds) { @@ -131,4 +137,12 @@ class OperationRule extends BaseRule implements IAuthRule { myAppliesToInstancesOfType = theAppliesToTypes; } + public void appliesToAnyInstance() { + myAppliesToAnyInstance = true; + } + + public void appliesToAnyType() { + myAppliesToAnyType = true; + } + } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java index cb3284c733f..404d3aebee2 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java @@ -371,6 +371,22 @@ public class RuleBuilder implements IAuthRuleBuilder { return appliesToTypes; } + @Override + public IAuthRuleFinished onAnyType() { + OperationRule rule = createRule(); + rule.appliesToAnyType(); + myRules.add(rule); + return new RuleBuilderFinished(); + } + + @Override + public IAuthRuleFinished onAnyInstance() { + OperationRule rule = createRule(); + rule.appliesToAnyInstance(); + myRules.add(rule); + return new RuleBuilderFinished(); + } + } } 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 05610639edc..a12186f07c2 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 @@ -99,12 +99,6 @@ public class AuthorizationInterceptorDstu2Test { return retVal; } - private IResource createPatient(Integer theId, int theVersion) { - IResource retVal = createPatient(theId); - retVal.setId(retVal.getId().withVersion(Integer.toString(theVersion))); - return retVal; - } - private IResource createPatient(Integer theId) { Patient retVal = new Patient(); if (theId != null) { @@ -113,6 +107,12 @@ public class AuthorizationInterceptorDstu2Test { retVal.addName().addFamily("FAM"); return retVal; } + + private IResource createPatient(Integer theId, int theVersion) { + IResource retVal = createPatient(theId); + retVal.setId(retVal.getId().withVersion(Integer.toString(theVersion))); + return retVal; + } private String extractResponseAndClose(HttpResponse status) throws IOException { if (status.getEntity() == null) { @@ -497,6 +497,46 @@ public class AuthorizationInterceptorDstu2Test { assertTrue(ourHitMethod); } + @Test + public void testHistoryWithReadAll() throws Exception { + ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { + @Override + public List buildRuleList(RequestDetails theRequestDetails) { + //@formatter:off + return new RuleBuilder() + .allow("Rule 1").read().allResources().withAnyId() + .build(); + //@formatter:on + } + }); + + HttpGet httpGet; + HttpResponse status; + + ourReturn = Arrays.asList(createPatient(2, 1)); + + ourHitMethod = false; + httpGet = new HttpGet("http://localhost:" + ourPort + "/_history"); + status = ourClient.execute(httpGet); + extractResponseAndClose(status); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertTrue(ourHitMethod); + + ourHitMethod = false; + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/_history"); + status = ourClient.execute(httpGet); + extractResponseAndClose(status); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertTrue(ourHitMethod); + + ourHitMethod = false; + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1/_history"); + status = ourClient.execute(httpGet); + extractResponseAndClose(status); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertTrue(ourHitMethod); + } + @Test public void testMetadataAllow() throws Exception { ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @@ -747,6 +787,75 @@ public class AuthorizationInterceptorDstu2Test { } + @Test + public void testOperationInstanceLevelAnyInstance() throws Exception { + ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { + @Override + public List buildRuleList(RequestDetails theRequestDetails) { + return new RuleBuilder() + .allow("RULE 1").operation().named("opName").onAnyInstance().andThen() + .build(); + } + }); + + HttpGet httpGet; + HttpResponse status; + String response; + + // Server + ourHitMethod = false; + ourReturn = Arrays.asList(createObservation(10, "Patient/2")); + httpGet = new HttpGet("http://localhost:" + ourPort + "/$opName"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + assertThat(response, containsString("Access denied by default policy")); + assertEquals(403, status.getStatusLine().getStatusCode()); + assertFalse(ourHitMethod); + + // Type + ourHitMethod = false; + ourReturn = Arrays.asList(createPatient(2)); + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/$opName"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + ourLog.info(response); + assertThat(response, containsString("Access denied by default policy")); + assertEquals(403, status.getStatusLine().getStatusCode()); + assertFalse(ourHitMethod); + + // Instance + ourHitMethod = false; + ourReturn = Arrays.asList(createPatient(2)); + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1/$opName"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + ourLog.info(response); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertTrue(ourHitMethod); + + // Another Instance + ourHitMethod = false; + ourReturn = Arrays.asList(createPatient(2)); + httpGet = new HttpGet("http://localhost:" + ourPort + "/Observation/2/$opName"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + ourLog.info(response); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertTrue(ourHitMethod); + + // Wrong name + ourHitMethod = false; + ourReturn = Arrays.asList(createPatient(2)); + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/2/$opName2"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + ourLog.info(response); + assertThat(response, containsString("Access denied by default policy")); + assertEquals(403, status.getStatusLine().getStatusCode()); + assertFalse(ourHitMethod); + + } + @Test public void testOperationNotAllowedWithWritePermissiom() throws Exception { ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @@ -926,13 +1035,13 @@ public class AuthorizationInterceptorDstu2Test { } @Test - public void testHistoryWithReadAll() throws Exception { + public void testOperationTypeLevelWildcard() throws Exception { ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @Override public List buildRuleList(RequestDetails theRequestDetails) { //@formatter:off return new RuleBuilder() - .allow("Rule 1").read().allResources().withAnyId() + .allow("RULE 1").operation().named("opName").onAnyType().andThen() .build(); //@formatter:on } @@ -940,29 +1049,59 @@ public class AuthorizationInterceptorDstu2Test { HttpGet httpGet; HttpResponse status; + String response; - ourReturn = Arrays.asList(createPatient(2, 1)); - + // Server ourHitMethod = false; - httpGet = new HttpGet("http://localhost:" + ourPort + "/_history"); + ourReturn = Arrays.asList(createObservation(10, "Patient/2")); + httpGet = new HttpGet("http://localhost:" + ourPort + "/$opName"); status = ourClient.execute(httpGet); - extractResponseAndClose(status); + response = extractResponseAndClose(status); + assertThat(response, containsString("Access denied by default policy")); + assertEquals(403, status.getStatusLine().getStatusCode()); + assertFalse(ourHitMethod); + + // Type + ourHitMethod = false; + ourReturn = Arrays.asList(createPatient(2)); + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/$opName"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + ourLog.info(response); assertEquals(200, status.getStatusLine().getStatusCode()); assertTrue(ourHitMethod); + // Another type ourHitMethod = false; - httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/_history"); + ourReturn = Arrays.asList(createPatient(2)); + httpGet = new HttpGet("http://localhost:" + ourPort + "/Observation/$opName"); status = ourClient.execute(httpGet); - extractResponseAndClose(status); + response = extractResponseAndClose(status); + ourLog.info(response); assertEquals(200, status.getStatusLine().getStatusCode()); assertTrue(ourHitMethod); + // Wrong name ourHitMethod = false; - httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1/_history"); + ourReturn = Arrays.asList(createPatient(2)); + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/$opName2"); status = ourClient.execute(httpGet); - extractResponseAndClose(status); - assertEquals(200, status.getStatusLine().getStatusCode()); - assertTrue(ourHitMethod); + response = extractResponseAndClose(status); + ourLog.info(response); + assertThat(response, containsString("Access denied by default policy")); + assertEquals(403, status.getStatusLine().getStatusCode()); + assertFalse(ourHitMethod); + + // Instance + ourHitMethod = false; + ourReturn = Arrays.asList(createPatient(2)); + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1/$opName"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + ourLog.info(response); + assertThat(response, containsString("Access denied by default policy")); + assertEquals(403, status.getStatusLine().getStatusCode()); + assertFalse(ourHitMethod); } @Test @@ -1711,19 +1850,6 @@ public class AuthorizationInterceptorDstu2Test { return retVal; } - @History() - public List history() { - ourHitMethod = true; - return (ourReturn); - } - - @History() - public List history(@IdParam IdDt theId) { - ourHitMethod = true; - return (ourReturn); - } - - @Delete() public MethodOutcome delete(IRequestOperationCallback theRequestOperationCallback, @IdParam IdDt theId, @ConditionalUrlParam String theConditionalUrl, RequestDetails theRequestDetails) { ourHitMethod = true; @@ -1744,11 +1870,24 @@ public class AuthorizationInterceptorDstu2Test { return retVal; } + @Override public Class getResourceType() { return Patient.class; } + @History() + public List history() { + ourHitMethod = true; + return (ourReturn); + } + + @History() + public List history(@IdParam IdDt theId) { + ourHitMethod = true; + return (ourReturn); + } + @Operation(name = "opName", idempotent = true) public Parameters operation() { ourHitMethod = true; @@ -1767,6 +1906,12 @@ public class AuthorizationInterceptorDstu2Test { return (Parameters) new Parameters().setId("1"); } + @Operation(name = "opName2", idempotent = true) + public Parameters operation2() { + ourHitMethod = true; + return (Parameters) new Parameters().setId("1"); + } + @Read(version = true) public Patient read(@IdParam IdDt theId) { ourHitMethod = true; @@ -1821,6 +1966,12 @@ public class AuthorizationInterceptorDstu2Test { public static class PlainProvider { + @History() + public List history() { + ourHitMethod = true; + return (ourReturn); + } + @Operation(name = "opName", idempotent = true) public Parameters operation() { ourHitMethod = true; @@ -1833,12 +1984,6 @@ public class AuthorizationInterceptorDstu2Test { return (Bundle) ourReturn.get(0); } - @History() - public List history() { - ourHitMethod = true; - return (ourReturn); - } - } diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 36e62161940..d0d475e21a7 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -166,6 +166,10 @@ JPA server interceptor methods for create/update/delete provided the wrong version ID to the interceptors + + AuthorizationInterceptor can now authorize (allow/deny) extended operations + on instances and types by wildcard (on any type, or on any instance) +