honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Allow AuthorizationInterceptor to properly authorize GraphQL statements (#1864) 

* Allow AuthorizationInterceptor to properly authorize GraphQL statements

* Add changelog
This commit is contained in:
James Agnew 2020-05-22 20:40:37 -04:00 committed by GitHub
parent 12f80b3da5
commit c374383b37
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 70 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/1864-allow-authinterceptor-graphql.yaml Normal file
View File

@ -0,0 +1,6 @@
---
type: add
issue: 1864
title: "AuthorizationInterceptor can now fully secure GraphQL operations in the JPA server, meaning that
it is possible to use compartment and type rules to restrict the specific data that is available through
the GraphQL interface."

47
hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorJpaR4Test.java
View File

@ -17,8 +17,11 @@ import ca.uhn.fhir.rest.server.interceptor.auth.PolicyEnum;
import ca.uhn.fhir.rest.server.interceptor.auth.RuleBuilder;
import ca.uhn.fhir.rest.server.provider.ProviderConstants;
import ca.uhn.fhir.util.TestUtil;
import ca.uhn.fhir.util.UrlUtil;
import org.apache.commons.io.IOUtils;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpDelete;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
@ -45,9 +48,11 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.List;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
@ -757,7 +762,7 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes
diff = ourClient.operation().onInstance("Patient/A").named(ProviderConstants.DIFF_OPERATION_NAME).withNoParameters(Parameters.class).execute();
assertEquals(1, diff.getParameter().size());
diff = ourClient.operation().onInstanceVersion(new IdType("Patient/A/_history/2")).named(ProviderConstants.DIFF_OPERATION_NAME).withNoParameters(Parameters.class).execute();
diff = ourClient.operation().onInstanceVersion(new IdType("Patient/A/_history/2")).named(ProviderConstants.DIFF_OPERATION_NAME).withNoParameters(Parameters.class).execute();
assertEquals(1, diff.getParameter().size());
try {
@ -795,7 +800,7 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes
.onServer()
.named(ProviderConstants.DIFF_OPERATION_NAME)
.withParameter(Parameters.class, ProviderConstants.DIFF_FROM_PARAMETER, new StringType("Patient/A"))
.andParameter( ProviderConstants.DIFF_TO_PARAMETER, new StringType("Patient/B"))
.andParameter(ProviderConstants.DIFF_TO_PARAMETER, new StringType("Patient/B"))
.execute();
assertEquals(2, diff.getParameter().size());
@ -805,7 +810,7 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes
.onServer()
.named(ProviderConstants.DIFF_OPERATION_NAME)
.withParameter(Parameters.class, ProviderConstants.DIFF_FROM_PARAMETER, new StringType("Observation/C"))
.andParameter( ProviderConstants.DIFF_TO_PARAMETER, new StringType("Observation/D"))
.andParameter(ProviderConstants.DIFF_TO_PARAMETER, new StringType("Observation/D"))
.execute();
fail();
} catch (ForbiddenOperationException e) {
@ -814,6 +819,42 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes
}
@Test
public void testGraphQL_AllowedByType_Instance() throws IOException {
createPatient(withId("A"), withFamily("MY_FAMILY"));
createPatient(withId("B"), withFamily("MY_FAMILY"));
ourRestServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {
@Override
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
return new RuleBuilder()
.allow().graphQL().any().andThen()
.allow().read().instance("Patient/A").andThen()
.denyAll()
.build();
}
});
HttpGet httpGet;
String query = "{name{family,given}}";
httpGet = new HttpGet(ourServerBase + "/Patient/A/$graphql?query=" + UrlUtil.escapeUrlParam(query));
try (CloseableHttpResponse response = ourHttpClient.execute(httpGet)) {
String resp = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8);
assertEquals(200, response.getStatusLine().getStatusCode());
assertThat(resp, containsString("MY_FAMILY"));
}
httpGet = new HttpGet(ourServerBase + "/Patient/B/$graphql?query=" + UrlUtil.escapeUrlParam(query));
try (CloseableHttpResponse response = ourHttpClient.execute(httpGet)) {
String resp = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8);
assertEquals(403, response.getStatusLine().getStatusCode());
}
}
/**
* See #762
*/

2
hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java
View File

@ -208,7 +208,7 @@ public class AuthorizationInterceptor implements IRuleApplier {
return OperationExamineDirection.NONE;
case GRAPHQL_REQUEST:
return OperationExamineDirection.IN;
return OperationExamineDirection.BOTH;
default:
// Should not happen

6
hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java
View File

@ -20,6 +20,7 @@ package ca.uhn.fhir.rest.server.interceptor.auth;
* #L%
*/
import ca.uhn.fhir.interceptor.api.Pointcut;
import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.Verdict;
@ -102,4 +103,9 @@ abstract class BaseRule implements IAuthRule {
}
return new Verdict(myMode, this);
}
protected boolean isResourceAccess(Pointcut thePointcut) {
return thePointcut.equals(Pointcut.STORAGE_PREACCESS_RESOURCES) || thePointcut.equals(Pointcut.STORAGE_PRESHOW_RESOURCES);
}
}

5
hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderGraphQL.java
View File

@ -23,9 +23,8 @@ package ca.uhn.fhir.rest.server.interceptor.auth;
public interface IAuthRuleBuilderGraphQL {
/**
* Note that this is an all-or-nothing grant for now, it
* is not yet possible to specify individual resource security when
* using GraphQL.
* Allow any GraphQL request. Note that this does not mean that any specific sub-operations are permitted, as
* other rules may be required in order to actually access specific resources
*/
IAuthRuleFinished any();
}

3
hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/OperationRule.java
View File

@ -87,8 +87,7 @@ class OperationRule extends BaseRule implements IAuthRule {
// Operation rules apply to the execution of the operation itself, not to side effects like
// loading resources (that will presumably be reflected in the response). Those loads need
// to be explicitly authorized
boolean isResourceAccess = thePointcut.equals(Pointcut.STORAGE_PREACCESS_RESOURCES) || thePointcut.equals(Pointcut.STORAGE_PRESHOW_RESOURCES);
if (isResourceAccess) {
if (isResourceAccess(thePointcut)) {
return null;
}

14
hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java
View File

@ -240,7 +240,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
// so just let it through for now..
return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource);
}
if (theInputResource== null && myClassifierCompartmentOwners != null && myClassifierCompartmentOwners.size() > 0) {
if (theInputResource == null && myClassifierCompartmentOwners != null && myClassifierCompartmentOwners.size() > 0) {
return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource);
}
@ -252,6 +252,12 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
break;
case GRAPHQL:
if (theOperation == RestOperationTypeEnum.GRAPHQL_REQUEST) {
// Make sure that the requestor actually has sufficient access to see the given resource
if (isResourceAccess(thePointcut)) {
return null;
}
return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource);
} else {
return null;
@ -276,8 +282,8 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
UrlUtil.UrlParts parts = UrlUtil.parseUrl(nextPart.getUrl());
inputResourceId = theRequestDetails.getFhirContext().getVersion().newIdType();
inputResourceId.setParts(null, parts.getResourceType(), parts.getResourceId(), null);
inputResourceId = theRequestDetails.getFhirContext().getVersion().newIdType();
inputResourceId.setParts(null, parts.getResourceType(), parts.getResourceId(), null);
}
RestOperationTypeEnum operation;
@ -511,7 +517,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
return verdict;
}
}
} else if (getMode() == PolicyEnum.ALLOW){
} else if (getMode() == PolicyEnum.ALLOW) {
return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource);
}
break;