Fix a big with the new permissions

This commit is contained in:
James Agnew 2017-03-24 15:33:47 +08:00
parent 44c0075409
commit 69748538d6
4 changed files with 140 additions and 62 deletions

View File

@ -10,7 +10,7 @@ package ca.uhn.fhir.rest.server.interceptor.auth;
* 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
* 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,
@ -30,6 +30,8 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.commons.lang3.Validate;
import org.apache.commons.lang3.builder.ToStringBuilder;
import org.apache.commons.lang3.builder.ToStringStyle;
import org.hl7.fhir.instance.model.api.IBaseBundle;
import org.hl7.fhir.instance.model.api.IBaseParameters;
import org.hl7.fhir.instance.model.api.IBaseResource;
@ -51,7 +53,7 @@ import ca.uhn.fhir.util.CoverageIgnore;
* inspect requests and responses to determine whether the calling user
* has permission to perform the given action.
* <p>
* See the HAPI FHIR
* See the HAPI FHIR
* <a href="http://jamesagnew.github.io/hapi-fhir/doc_rest_server_security.html">Documentation on Server Security</a>
* for information on how to use this interceptor.
* </p>
@ -59,7 +61,7 @@ import ca.uhn.fhir.util.CoverageIgnore;
public class AuthorizationInterceptor extends InterceptorAdapter implements IServerOperationInterceptor, IRuleApplier {
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(AuthorizationInterceptor.class);
private PolicyEnum myDefaultPolicy = PolicyEnum.DENY;
/**
@ -68,29 +70,32 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
public AuthorizationInterceptor() {
super();
}
/**
* Constructor
*
* @param theDefaultPolicy The default policy if no rules apply (must not be null)
* @param theDefaultPolicy
* The default policy if no rules apply (must not be null)
*/
public AuthorizationInterceptor(PolicyEnum theDefaultPolicy) {
this();
setDefaultPolicy(theDefaultPolicy);
}
private void applyRulesAndFailIfDeny(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, IIdType theInputResourceId, IBaseResource theOutputResource) {
private void applyRulesAndFailIfDeny(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, IIdType theInputResourceId,
IBaseResource theOutputResource) {
Verdict decision = applyRulesAndReturnDecision(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource);
if (decision.getDecision() == PolicyEnum.ALLOW) {
return;
}
handleDeny(decision);
}
@Override
public Verdict applyRulesAndReturnDecision(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, IIdType theInputResourceId, IBaseResource theOutputResource) {
public Verdict applyRulesAndReturnDecision(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, IIdType theInputResourceId,
IBaseResource theOutputResource) {
List<IAuthRule> rules = buildRuleList(theRequestDetails);
ourLog.trace("Applying {} rules to render an auth decision for operation {}", rules.size(), theOperation);
@ -102,12 +107,12 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
break;
}
}
if (verdict == null) {
ourLog.trace("No rules returned a decision, applying default {}", myDefaultPolicy);
return new Verdict(myDefaultPolicy, null);
}
return verdict;
}
@ -117,16 +122,16 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
* <p>
* Typically this is done by examining <code>theRequestDetails</code> to find
* out who the current user is and then using a {@link RuleBuilder} to create
* an appropriate rule chain.
* an appropriate rule chain.
* </p>
*
* @param theRequestDetails The individual request currently being applied
* @param theRequestDetails
* The individual request currently being applied
*/
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
return new ArrayList<IAuthRule>();
}
private OperationExamineDirection determineOperationDirection(RestOperationTypeEnum theOperation, IBaseResource theRequestResource) {
switch (theOperation) {
case ADD_TAGS:
@ -134,29 +139,29 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
case GET_TAGS:
// These are DSTU1 operations and not relevant
return OperationExamineDirection.NONE;
case EXTENDED_OPERATION_INSTANCE:
case EXTENDED_OPERATION_SERVER:
case EXTENDED_OPERATION_TYPE:
return OperationExamineDirection.BOTH;
case METADATA:
// Security does not apply to these operations
// Security does not apply to these operations
return OperationExamineDirection.IN;
case DELETE:
// Delete is a special case
return OperationExamineDirection.NONE;
case CREATE:
case UPDATE:
// if (theRequestResource != null) {
// if (theRequestResource.getIdElement() != null) {
// if (theRequestResource.getIdElement().hasIdPart() == false) {
// return OperationExamineDirection.IN_UNCATEGORIZED;
// }
// }
// }
// if (theRequestResource != null) {
// if (theRequestResource.getIdElement() != null) {
// if (theRequestResource.getIdElement().hasIdPart() == false) {
// return OperationExamineDirection.IN_UNCATEGORIZED;
// }
// }
// }
return OperationExamineDirection.IN;
case META:
@ -164,7 +169,7 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
case META_DELETE:
// meta operations do not apply yet
return OperationExamineDirection.NONE;
case GET_PAGE:
case HISTORY_INSTANCE:
case HISTORY_SYSTEM:
@ -174,14 +179,14 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
case SEARCH_TYPE:
case VREAD:
return OperationExamineDirection.OUT;
case TRANSACTION:
return OperationExamineDirection.BOTH;
case VALIDATE:
// Nothing yet
return OperationExamineDirection.NONE;
default:
// Should not happen
throw new IllegalStateException("Unable to apply security to event of type " + theOperation);
@ -199,7 +204,7 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
/**
* Handle an access control verdict of {@link PolicyEnum#DENY}.
* <p>
* Subclasses may override to implement specific behaviour, but default is to
* Subclasses may override to implement specific behaviour, but default is to
* throw {@link ForbiddenOperationException} (HTTP 403) with error message citing the
* rule name which trigered failure
* </p>
@ -211,16 +216,16 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
}
throw new ForbiddenOperationException("Access denied by default policy (no applicable rules)");
}
private void handleUserOperation(RequestDetails theRequest, IBaseResource theResource, RestOperationTypeEnum operation) {
applyRulesAndFailIfDeny(operation, theRequest, theResource, theResource.getIdElement(), null);
}
@Override
public void incomingRequestPreHandled(RestOperationTypeEnum theOperation, ActionRequestDetails theProcessedRequest) {
IBaseResource inputResource = null;
IIdType inputResourceId = null;
switch (determineOperationDirection(theOperation, theProcessedRequest.getResource())) {
case IN:
case BOTH:
@ -228,7 +233,7 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
inputResourceId = theProcessedRequest.getId();
break;
case OUT:
// inputResource = null;
// inputResource = null;
inputResourceId = theProcessedRequest.getId();
break;
case NONE:
@ -238,7 +243,7 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
RequestDetails requestDetails = theProcessedRequest.getRequestDetails();
applyRulesAndFailIfDeny(theOperation, requestDetails, inputResource, inputResourceId, null);
}
@Override
@CoverageIgnore
public boolean outgoingResponse(RequestDetails theRequestDetails, Bundle theBundle) {
@ -247,7 +252,8 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
@Override
@CoverageIgnore
public boolean outgoingResponse(RequestDetails theRequestDetails, Bundle theResponseObject, HttpServletRequest theServletRequest, HttpServletResponse theServletResponse) throws AuthenticationException {
public boolean outgoingResponse(RequestDetails theRequestDetails, Bundle theResponseObject, HttpServletRequest theServletRequest, HttpServletResponse theServletResponse)
throws AuthenticationException {
throw failForDstu1();
}
@ -264,7 +270,7 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
FhirContext fhirContext = theRequestDetails.getServer().getFhirContext();
List<IBaseResource> resources = Collections.emptyList();
switch (theRequestDetails.getRestOperationType()) {
case SEARCH_SYSTEM:
case SEARCH_TYPE:
@ -277,8 +283,8 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
case EXTENDED_OPERATION_INSTANCE: {
if (theResponseObject != null) {
if (theResponseObject instanceof IBaseBundle) {
// IBaseBundle responseBundle = (IBaseBundle) theResponseObject;
// resources = toListOfResources(fhirContext, responseBundle);
// IBaseBundle responseBundle = (IBaseBundle) theResponseObject;
// resources = toListOfResources(fhirContext, responseBundle);
resources = toListOfResourcesAndExcludeContainer(theResponseObject, fhirContext);
} else if (theResponseObject instanceof IBaseParameters) {
resources = toListOfResourcesAndExcludeContainer(theResponseObject, fhirContext);
@ -293,7 +299,7 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
break;
}
}
for (IBaseResource nextResponse : resources) {
applyRulesAndFailIfDeny(theRequestDetails.getRestOperationType(), theRequestDetails, null, null, nextResponse);
}
@ -309,7 +315,8 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
@CoverageIgnore
@Override
public boolean outgoingResponse(RequestDetails theRequestDetails, TagList theResponseObject, HttpServletRequest theServletRequest, HttpServletResponse theServletResponse) throws AuthenticationException {
public boolean outgoingResponse(RequestDetails theRequestDetails, TagList theResponseObject, HttpServletRequest theServletRequest, HttpServletResponse theServletResponse)
throws AuthenticationException {
throw failForDstu1();
}
@ -331,7 +338,8 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
/**
* The default policy if no rules have been found to apply. Default value for this setting is {@link PolicyEnum#DENY}
*
* @param theDefaultPolicy The policy (must not be <code>null</code>)
* @param theDefaultPolicy
* The policy (must not be <code>null</code>)
*/
public void setDefaultPolicy(PolicyEnum theDefaultPolicy) {
Validate.notNull(theDefaultPolicy, "theDefaultPolicy must not be null");
@ -341,37 +349,34 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
private List<IBaseResource> toListOfResourcesAndExcludeContainer(IBaseResource theResponseObject, FhirContext fhirContext) {
List<IBaseResource> 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<IBaseResource> toListOfResources(FhirContext fhirContext, IBaseBundle responseBundle) {
// List<IBaseResource> retVal = BundleUtil.toListOfResources(fhirContext, responseBundle);
// for (int i = 0; i < retVal.size(); i++) {
// IBaseResource nextResource = retVal.get(i);
// if (nextResource instanceof IBaseBundle) {
// retVal.addAll(BundleUtil.toListOfResources(fhirContext, (IBaseBundle) nextResource));
// retVal.remove(i);
// i--;
// }
// }
// return retVal;
// }
// private List<IBaseResource> toListOfResources(FhirContext fhirContext, IBaseBundle responseBundle) {
// List<IBaseResource> retVal = BundleUtil.toListOfResources(fhirContext, responseBundle);
// for (int i = 0; i < retVal.size(); i++) {
// IBaseResource nextResource = retVal.get(i);
// if (nextResource instanceof IBaseBundle) {
// retVal.addAll(BundleUtil.toListOfResources(fhirContext, (IBaseBundle) nextResource));
// retVal.remove(i);
// i--;
// }
// }
// return retVal;
// }
private static UnsupportedOperationException failForDstu1() {
return new UnsupportedOperationException("Use of this interceptor on DSTU1 servers is not supportd");
}
private enum OperationExamineDirection {
BOTH,
IN,
NONE,
OUT,
BOTH, IN, NONE, OUT,
}
public static class Verdict {
@ -391,7 +396,15 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
public PolicyEnum getDecision() {
return myDecision;
}
@Override
public String toString() {
ToStringBuilder b = new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE);
b.append("rule", myDecidingRule.getName());
b.append("decision", myDecision.name());
return b.build();
}
}
}

View File

@ -87,6 +87,9 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
}
}
appliesToResource = theOutputResource;
if (theOutputResource != null) {
appliesToResourceId = theOutputResource.getIdElement();
}
break;
case WRITE:
if (theInputResource == null && theInputResourceId == null) {

View File

@ -0,0 +1,12 @@
package ca.uhn.fhir.rest.server.interceptor.auth;
import ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.Verdict;
public class VerdictTest {
public void testToString() {
Verdict v = new AuthorizationInterceptor.Verdict(PolicyEnum.ALLOW, new RuleImplOp("foo"));
v.toString();
}
}

View File

@ -1777,6 +1777,56 @@ public class AuthorizationInterceptorDstu2Test {
assertFalse(ourHitMethod);
}
@Test
public void testReadByInstance() throws Exception {
ourConditionalCreateId = "1";
ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {
@Override
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
//@formatter:off
return new RuleBuilder()
.allow("Rule 1").read().instance("Observation/900").andThen()
.allow("Rule 1").read().instance("901").andThen()
.build();
//@formatter:on
}
});
HttpResponse status;
String response;
HttpGet httpGet;
ourReturn = Arrays.asList(createObservation(900, "Patient/1"));
ourHitMethod = false;
httpGet = new HttpGet("http://localhost:" + ourPort + "/Observation/900");
status = ourClient.execute(httpGet);
response = extractResponseAndClose(status);
assertEquals(200, status.getStatusLine().getStatusCode());
assertTrue(ourHitMethod);
ourReturn = Arrays.asList(createPatient(901));
ourHitMethod = false;
httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/901");
status = ourClient.execute(httpGet);
response = extractResponseAndClose(status);
assertEquals(200, status.getStatusLine().getStatusCode());
assertTrue(ourHitMethod);
ourReturn = Arrays.asList(createPatient(1));
ourHitMethod = false;
httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1?_format=json");
status = ourClient.execute(httpGet);
response = extractResponseAndClose(status);
assertEquals(403, status.getStatusLine().getStatusCode());
assertEquals(ERR403, response);
assertFalse(ourHitMethod);
}
@AfterClass
public static void afterClassClearContext() throws Exception {
ourServer.stop();