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 37cc777853a..e5c69a43ea3 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 @@ -221,20 +221,16 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer IIdType inputResourceId = null; switch (determineOperationDirection(theOperation, theProcessedRequest.getResource())) { - case IN_UNCATEGORIZED: - inputResourceId = theProcessedRequest.getId(); - if (inputResourceId == null || inputResourceId.hasIdPart() == false) { - return; - } else { - break; - } case IN: case BOTH: inputResource = theProcessedRequest.getResource(); inputResourceId = theProcessedRequest.getId(); break; - case NONE: case OUT: + inputResource = null; + inputResourceId = theProcessedRequest.getId(); + break; + case NONE: return; } @@ -257,8 +253,6 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer @Override public boolean outgoingResponse(RequestDetails theRequestDetails, IBaseResource theResponseObject) { switch (determineOperationDirection(theRequestDetails.getRestOperationType(), null)) { - case IN_UNCATEGORIZED: - return true; case IN: case NONE: return true; @@ -354,7 +348,6 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer private enum OperationExamineDirection { IN, - IN_UNCATEGORIZED, NONE, OUT, BOTH, diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java index 05af00d3d98..8e5a7c6a7b0 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java @@ -63,7 +63,16 @@ class RuleImplOp extends BaseRule implements IAuthRule { switch (myOp) { case READ: if (theOutputResource == null) { - return null; + switch (theOperation) { + case READ: + appliesToResourceId = theInputResourceId; + break; + case SEARCH_SYSTEM: + case SEARCH_TYPE: + return new Verdict(PolicyEnum.ALLOW, this); + default: + return null; + } } appliesToResource = theOutputResource; break; 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 f5daeb3ff33..583ad57cbbc 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 @@ -167,7 +167,7 @@ public class AuthorizationInterceptorDstu2Test { ourLog.info(response); assertThat(response, containsString("Access denied by rule: Rule 1")); assertEquals(403, status.getStatusLine().getStatusCode()); - assertTrue(ourHitMethod); + assertFalse(ourHitMethod); ourHitMethod = false; httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1/$validate"); @@ -321,7 +321,7 @@ public class AuthorizationInterceptorDstu2Test { ourLog.info(response); assertThat(response, containsString("Access denied by rule: Default Rule")); assertEquals(403, status.getStatusLine().getStatusCode()); - assertTrue(ourHitMethod); + assertFalse(ourHitMethod); ourHitMethod = false; httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1/$validate"); @@ -625,7 +625,7 @@ public class AuthorizationInterceptorDstu2Test { ourLog.info(response); assertThat(response, containsString("Access denied by default policy (no applicable rules)")); assertEquals(403, status.getStatusLine().getStatusCode()); - assertTrue(ourHitMethod); + assertFalse(ourHitMethod); ourReturn = Arrays.asList(createPatient(1), createObservation(10, "Patient/2")); ourHitMethod = false; @@ -679,8 +679,8 @@ public class AuthorizationInterceptorDstu2Test { httpGet = new HttpGet("http://localhost:" + ourPort + "/Observation/10"); status = ourClient.execute(httpGet); extractResponseAndClose(status); - assertEquals(200, status.getStatusLine().getStatusCode()); - assertTrue(ourHitMethod); + assertEquals(403, status.getStatusLine().getStatusCode()); + assertFalse(ourHitMethod); ourReturn = Arrays.asList(createPatient(1), createObservation(10, "Patient/1")); ourHitMethod = false; @@ -719,7 +719,7 @@ public class AuthorizationInterceptorDstu2Test { ourLog.info(response); assertThat(response, containsString("Access denied by default policy (no applicable rules)")); assertEquals(403, status.getStatusLine().getStatusCode()); - assertTrue(ourHitMethod); + assertFalse(ourHitMethod); ourReturn = Arrays.asList(createObservation(10, "Patient/2")); ourHitMethod = false; @@ -729,7 +729,7 @@ public class AuthorizationInterceptorDstu2Test { ourLog.info(response); assertThat(response, containsString("Access denied by default policy (no applicable rules)")); assertEquals(403, status.getStatusLine().getStatusCode()); - assertTrue(ourHitMethod); + assertFalse(ourHitMethod); ourReturn = Arrays.asList(createPatient(1), createObservation(10, "Patient/2")); ourHitMethod = false; diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 1e8afdb9d8b..870fda86fca 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -221,6 +221,12 @@ the generated bundle. Thanks to Hannes Venter for the pull request and contribution! + + AuthorizationInterceptor is now a bit more aggressive + at blocking read operations, stopping them on the + way in if there is no way they will be accepted + to the resource check on the way out +