Fix #667 - When using the AuthorizationInterceptor with the JPA server, when a client is updating a resource from A to B, the user now needs to have write permission for both A and B
This commit is contained in:
parent
14edc79ac1
commit
c2e5fa3f18
|
@ -34,7 +34,17 @@ public interface IRequestOperationCallback {
|
|||
|
||||
void resourcesDeleted(Collection<? extends IBaseResource> theResource);
|
||||
|
||||
/**
|
||||
* @deprecated Deprecated in HAPI FHIR 2.6 - Use {@link IRequestOperationCallback#resourceUpdated(IBaseResource, IBaseResource)} instead
|
||||
*/
|
||||
@Deprecated
|
||||
void resourcesUpdated(Collection<? extends IBaseResource> theResource);
|
||||
|
||||
/**
|
||||
* @deprecated Deprecated in HAPI FHIR 2.6 - Use {@link IRequestOperationCallback#resourceUpdated(IBaseResource, IBaseResource)} instead
|
||||
*/
|
||||
@Deprecated
|
||||
void resourceUpdated(IBaseResource theResource);
|
||||
|
||||
void resourceUpdated(IBaseResource theOldResource, IBaseResource theNewResource);
|
||||
}
|
||||
|
|
|
@ -400,13 +400,21 @@ public abstract class RequestDetails {
|
|||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
/**
|
||||
* @deprecated Deprecated in HAPI FHIR 2.6 - Use {@link IRequestOperationCallback#resourceUpdated(IBaseResource, IBaseResource)} instead
|
||||
*/
|
||||
@Deprecated
|
||||
public void resourcesUpdated(Collection<? extends IBaseResource> theResource) {
|
||||
for (IBaseResource next : theResource) {
|
||||
resourceUpdated(next);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* @deprecated Deprecated in HAPI FHIR 2.6 - Use {@link IRequestOperationCallback#resourceUpdated(IBaseResource, IBaseResource)} instead
|
||||
*/
|
||||
@Deprecated
|
||||
@Override
|
||||
public void resourceUpdated(IBaseResource theResource) {
|
||||
for (IServerInterceptor next : getInterceptors()) {
|
||||
|
@ -416,6 +424,15 @@ public abstract class RequestDetails {
|
|||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public void resourceUpdated(IBaseResource theOldResource, IBaseResource theNewResource) {
|
||||
for (IServerInterceptor next : getInterceptors()) {
|
||||
if (next instanceof IServerOperationInterceptor) {
|
||||
((IServerOperationInterceptor) next).resourceUpdated(RequestDetails.this, theOldResource, theNewResource);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -10,7 +10,7 @@ package ca.uhn.fhir.rest.server.interceptor;
|
|||
* 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,
|
||||
|
@ -25,7 +25,7 @@ import org.hl7.fhir.instance.model.api.IBaseResource;
|
|||
import ca.uhn.fhir.rest.method.RequestDetails;
|
||||
|
||||
/**
|
||||
* Server interceptor with added methods which can be called within the lifecycle of
|
||||
* Server interceptor with added methods which can be called within the lifecycle of
|
||||
* write operations (create/update/delete) or within transaction and batch
|
||||
* operations that call these sub-operations.
|
||||
*
|
||||
|
@ -33,12 +33,6 @@ import ca.uhn.fhir.rest.method.RequestDetails;
|
|||
*/
|
||||
public interface IServerOperationInterceptor extends IServerInterceptor {
|
||||
|
||||
/**
|
||||
* User code may call this method to indicate to an interceptor that
|
||||
* a resource is being deleted
|
||||
*/
|
||||
void resourceDeleted(RequestDetails theRequest, IBaseResource theResource);
|
||||
|
||||
/**
|
||||
* User code may call this method to indicate to an interceptor that
|
||||
* a resource is being created
|
||||
|
@ -47,8 +41,29 @@ public interface IServerOperationInterceptor extends IServerInterceptor {
|
|||
|
||||
/**
|
||||
* User code may call this method to indicate to an interceptor that
|
||||
* a resource is being updated
|
||||
* a resource is being deleted
|
||||
*/
|
||||
void resourceDeleted(RequestDetails theRequest, IBaseResource theResource);
|
||||
|
||||
/**
|
||||
* User code may call this method to indicate to an interceptor that
|
||||
* a resource is being updated
|
||||
*
|
||||
* @deprecated Deprecated in HAPI FHIR 2.6 in favour of {@link #resourceUpdated(RequestDetails, IBaseResource, IBaseResource)}
|
||||
*/
|
||||
@Deprecated
|
||||
void resourceUpdated(RequestDetails theRequest, IBaseResource theResource);
|
||||
|
||||
/**
|
||||
* User code may call this method to indicate to an interceptor that
|
||||
* a resource is being updated
|
||||
*
|
||||
* @param theOldResource
|
||||
* The resource as it was before the update, or <code>null</code> if this is not available. Interceptors should be able to handle situations where this is null, since it is not always
|
||||
* convenient or possible to provide a value for this field, but servers should try to populate it.
|
||||
* @param theNewResource
|
||||
* The resource as it will be after the update
|
||||
*/
|
||||
void resourceUpdated(RequestDetails theRequest, IBaseResource theOldResource, IBaseResource theNewResource);
|
||||
|
||||
}
|
||||
|
|
|
@ -39,9 +39,18 @@ public class ServerOperationInterceptorAdapter extends InterceptorAdapter implem
|
|||
// nothing
|
||||
}
|
||||
|
||||
/**
|
||||
* @deprecated Deprecated in HAPI FHIR 2.6 in favour of {@link #resourceUpdated(RequestDetails, IBaseResource, IBaseResource)}
|
||||
*/
|
||||
@Deprecated
|
||||
@Override
|
||||
public void resourceUpdated(RequestDetails theRequest, IBaseResource theResource) {
|
||||
// nothing
|
||||
}
|
||||
|
||||
@Override
|
||||
public void resourceUpdated(RequestDetails theRequest, IBaseResource theOldResource, IBaseResource theNewResource) {
|
||||
// nothing
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -44,8 +44,7 @@ import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
|
|||
import ca.uhn.fhir.rest.method.RequestDetails;
|
||||
import ca.uhn.fhir.rest.server.exceptions.AuthenticationException;
|
||||
import ca.uhn.fhir.rest.server.exceptions.ForbiddenOperationException;
|
||||
import ca.uhn.fhir.rest.server.interceptor.IServerOperationInterceptor;
|
||||
import ca.uhn.fhir.rest.server.interceptor.InterceptorAdapter;
|
||||
import ca.uhn.fhir.rest.server.interceptor.*;
|
||||
import ca.uhn.fhir.util.CoverageIgnore;
|
||||
|
||||
/**
|
||||
|
@ -58,7 +57,7 @@ import ca.uhn.fhir.util.CoverageIgnore;
|
|||
* for information on how to use this interceptor.
|
||||
* </p>
|
||||
*/
|
||||
public class AuthorizationInterceptor extends InterceptorAdapter implements IServerOperationInterceptor, IRuleApplier {
|
||||
public class AuthorizationInterceptor extends ServerOperationInterceptorAdapter implements IRuleApplier {
|
||||
|
||||
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(AuthorizationInterceptor.class);
|
||||
|
||||
|
@ -330,8 +329,11 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
|
|||
}
|
||||
|
||||
@Override
|
||||
public void resourceUpdated(RequestDetails theRequest, IBaseResource theResource) {
|
||||
handleUserOperation(theRequest, theResource, RestOperationTypeEnum.UPDATE);
|
||||
public void resourceUpdated(RequestDetails theRequest, IBaseResource theOldResource, IBaseResource theNewResource) {
|
||||
if (theOldResource != null) {
|
||||
handleUserOperation(theRequest, theOldResource, RestOperationTypeEnum.UPDATE);
|
||||
}
|
||||
handleUserOperation(theRequest, theNewResource, RestOperationTypeEnum.UPDATE);
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -357,19 +359,6 @@ public class AuthorizationInterceptor extends InterceptorAdapter implements ISer
|
|||
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 static UnsupportedOperationException failForDstu1() {
|
||||
return new UnsupportedOperationException("Use of this interceptor on DSTU1 servers is not supportd");
|
||||
}
|
||||
|
|
|
@ -1125,6 +1125,8 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
|
|||
notifyInterceptors(RestOperationTypeEnum.UPDATE, requestDetails);
|
||||
}
|
||||
|
||||
IBaseResource oldResource = toResource(entity, false);
|
||||
|
||||
// Perform update
|
||||
ResourceTable savedEntity = updateEntity(theResource, entity, null, thePerformIndexing, thePerformIndexing, new Date(), theForceUpdateVersion, thePerformIndexing);
|
||||
|
||||
|
@ -1143,6 +1145,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
|
|||
// Notify interceptors
|
||||
if (theRequestDetails != null) {
|
||||
theRequestDetails.getRequestOperationCallback().resourceUpdated(theResource);
|
||||
theRequestDetails.getRequestOperationCallback().resourceUpdated(oldResource, theResource);
|
||||
for (IServerInterceptor next : getConfig().getInterceptors()) {
|
||||
if (next instanceof IJpaServerInterceptor) {
|
||||
((IJpaServerInterceptor) next).resourceUpdated(requestDetails, entity);
|
||||
|
@ -1152,6 +1155,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
|
|||
for (IServerInterceptor next : getConfig().getInterceptors()) {
|
||||
if (next instanceof IServerOperationInterceptor) {
|
||||
((IServerOperationInterceptor) next).resourceUpdated(theRequestDetails, theResource);
|
||||
((IServerOperationInterceptor) next).resourceUpdated(theRequestDetails, oldResource, theResource);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -59,10 +59,9 @@ import ca.uhn.fhir.rest.param.TokenParam;
|
|||
import ca.uhn.fhir.rest.server.Constants;
|
||||
import ca.uhn.fhir.rest.server.EncodingEnum;
|
||||
import ca.uhn.fhir.rest.server.IBundleProvider;
|
||||
import ca.uhn.fhir.rest.server.interceptor.IServerOperationInterceptor;
|
||||
import ca.uhn.fhir.rest.server.interceptor.InterceptorAdapter;
|
||||
import ca.uhn.fhir.rest.server.interceptor.*;
|
||||
|
||||
public class RestHookSubscriptionDstu2Interceptor extends InterceptorAdapter implements IServerOperationInterceptor {
|
||||
public class RestHookSubscriptionDstu2Interceptor extends ServerOperationInterceptorAdapter {
|
||||
|
||||
private static volatile ExecutorService executor;
|
||||
private final static int MAX_THREADS = 1;
|
||||
|
@ -367,14 +366,14 @@ public class RestHookSubscriptionDstu2Interceptor extends InterceptorAdapter imp
|
|||
* a rest-hook subscription
|
||||
*/
|
||||
@Override
|
||||
public void resourceUpdated(RequestDetails theRequest, IBaseResource theResource) {
|
||||
String resourceType = getResourceName(theResource);
|
||||
IIdType idType = theResource.getIdElement();
|
||||
public void resourceUpdated(RequestDetails theRequest, IBaseResource theOldResource, IBaseResource theNewResource) {
|
||||
String resourceType = getResourceName(theNewResource);
|
||||
IIdType idType = theNewResource.getIdElement();
|
||||
|
||||
ourLog.info("resource updated type: " + resourceType);
|
||||
|
||||
if (theResource instanceof Subscription) {
|
||||
Subscription subscription = (Subscription) theResource;
|
||||
if (theNewResource instanceof Subscription) {
|
||||
Subscription subscription = (Subscription) theNewResource;
|
||||
if (subscription.getChannel() != null && subscription.getChannel().getTypeElement().getValueAsEnum() == SubscriptionChannelTypeEnum.REST_HOOK) {
|
||||
removeLocalSubscription(subscription.getIdElement().getIdPart());
|
||||
|
||||
|
|
|
@ -57,10 +57,9 @@ import ca.uhn.fhir.rest.param.TokenParam;
|
|||
import ca.uhn.fhir.rest.server.Constants;
|
||||
import ca.uhn.fhir.rest.server.EncodingEnum;
|
||||
import ca.uhn.fhir.rest.server.IBundleProvider;
|
||||
import ca.uhn.fhir.rest.server.interceptor.IServerOperationInterceptor;
|
||||
import ca.uhn.fhir.rest.server.interceptor.InterceptorAdapter;
|
||||
import ca.uhn.fhir.rest.server.interceptor.*;
|
||||
|
||||
public class RestHookSubscriptionDstu3Interceptor extends InterceptorAdapter implements IServerOperationInterceptor {
|
||||
public class RestHookSubscriptionDstu3Interceptor extends ServerOperationInterceptorAdapter {
|
||||
|
||||
private static volatile ExecutorService executor;
|
||||
private final static int MAX_THREADS = 1;
|
||||
|
@ -368,14 +367,14 @@ public class RestHookSubscriptionDstu3Interceptor extends InterceptorAdapter imp
|
|||
* a rest-hook subscription
|
||||
*/
|
||||
@Override
|
||||
public void resourceUpdated(RequestDetails theRequest, IBaseResource theResource) {
|
||||
String resourceType = getResourceName(theResource);
|
||||
IIdType idType = theResource.getIdElement();
|
||||
public void resourceUpdated(RequestDetails theRequest, IBaseResource theOldResource, IBaseResource theNewResource) {
|
||||
String resourceType = getResourceName(theNewResource);
|
||||
IIdType idType = theNewResource.getIdElement();
|
||||
|
||||
ourLog.info("resource updated type: " + resourceType);
|
||||
|
||||
if (theResource instanceof Subscription) {
|
||||
Subscription subscription = (Subscription) theResource;
|
||||
if (theNewResource instanceof Subscription) {
|
||||
Subscription subscription = (Subscription) theNewResource;
|
||||
if (subscription.getChannel() != null && subscription.getChannel().getType() == Subscription.SubscriptionChannelType.RESTHOOK) {
|
||||
removeLocalSubscription(subscription.getIdElement().getIdPart());
|
||||
|
||||
|
|
|
@ -1,6 +1,15 @@
|
|||
|
||||
package ca.uhn.fhir.jpa.interceptor;
|
||||
|
||||
import javax.annotation.PostConstruct;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
|
||||
import org.hl7.fhir.dstu3.model.Subscription;
|
||||
import org.hl7.fhir.instance.model.api.IBaseResource;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.beans.factory.annotation.Qualifier;
|
||||
|
||||
/*-
|
||||
* #%L
|
||||
* HAPI FHIR JPA Server
|
||||
|
@ -23,25 +32,13 @@ package ca.uhn.fhir.jpa.interceptor;
|
|||
|
||||
import ca.uhn.fhir.jpa.dao.IFhirResourceDao;
|
||||
import ca.uhn.fhir.jpa.dao.IFhirResourceDaoSubscription;
|
||||
import ca.uhn.fhir.jpa.entity.ResourceTable;
|
||||
import ca.uhn.fhir.rest.api.RequestTypeEnum;
|
||||
import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
|
||||
import ca.uhn.fhir.rest.method.RequestDetails;
|
||||
import ca.uhn.fhir.rest.server.exceptions.AuthenticationException;
|
||||
import ca.uhn.fhir.rest.server.interceptor.IServerOperationInterceptor;
|
||||
import ca.uhn.fhir.rest.server.interceptor.InterceptorAdapter;
|
||||
import org.hl7.fhir.dstu3.model.Subscription;
|
||||
import org.hl7.fhir.instance.model.api.IBaseResource;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.beans.factory.annotation.Qualifier;
|
||||
import ca.uhn.fhir.rest.server.interceptor.ServerOperationInterceptorAdapter;
|
||||
|
||||
import javax.annotation.PostConstruct;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
|
||||
public class WebSocketSubscriptionDstu3Interceptor extends InterceptorAdapter implements IServerOperationInterceptor {
|
||||
public class WebSocketSubscriptionDstu3Interceptor extends ServerOperationInterceptorAdapter {
|
||||
|
||||
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(WebSocketSubscriptionDstu3Interceptor.class);
|
||||
|
||||
|
@ -104,7 +101,7 @@ public class WebSocketSubscriptionDstu3Interceptor extends InterceptorAdapter im
|
|||
}
|
||||
|
||||
@Override
|
||||
public void resourceUpdated(RequestDetails theRequest, IBaseResource theResource) {
|
||||
public void resourceUpdated(RequestDetails theRequest, IBaseResource theOldResource, IBaseResource theNewResource) {
|
||||
// nothing
|
||||
}
|
||||
}
|
||||
|
|
|
@ -91,7 +91,7 @@ public class FhirResourceDaoDstu2InterceptorTest extends BaseJpaDstu2Test {
|
|||
tableCapt = ArgumentCaptor.forClass(ResourceTable.class);
|
||||
verify(myJpaInterceptor, times(1)).resourceCreated(detailsCapt.capture(), tableCapt.capture());
|
||||
verify(myJpaInterceptor, times(0)).resourceUpdated(detailsCapt.capture(), tableCapt.capture());
|
||||
|
||||
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -366,10 +366,10 @@ public class FhirResourceDaoDstu2InterceptorTest extends BaseJpaDstu2Test {
|
|||
doAnswer(new Answer<Void>() {
|
||||
@Override
|
||||
public Void answer(InvocationOnMock theInvocation) throws Throwable {
|
||||
IBaseResource res = (IBaseResource) theInvocation.getArguments()[0];
|
||||
IBaseResource res = (IBaseResource) theInvocation.getArguments()[1];
|
||||
assertEquals("Patient/" + id + "/_history/2", res.getIdElement().getValue());
|
||||
return null;
|
||||
}}).when(myRequestOperationCallback).resourceUpdated(any(IBaseResource.class));
|
||||
}}).when(myRequestOperationCallback).resourceUpdated(any(IBaseResource.class), any(IBaseResource.class));
|
||||
|
||||
Bundle xactBundle = new Bundle();
|
||||
xactBundle.setType(BundleTypeEnum.TRANSACTION);
|
||||
|
@ -385,6 +385,7 @@ public class FhirResourceDaoDstu2InterceptorTest extends BaseJpaDstu2Test {
|
|||
assertEquals(2L, newId.getVersionIdPartAsLong().longValue());
|
||||
|
||||
verify(myRequestOperationCallback, times(1)).resourceUpdated(any(IBaseResource.class));
|
||||
verify(myRequestOperationCallback, times(1)).resourceUpdated(any(IBaseResource.class), any(IBaseResource.class));
|
||||
verify(myRequestOperationCallback, times(1)).resourceCreated(any(IBaseResource.class));
|
||||
verifyNoMoreInteractions(myRequestOperationCallback);
|
||||
}
|
||||
|
@ -398,10 +399,10 @@ public class FhirResourceDaoDstu2InterceptorTest extends BaseJpaDstu2Test {
|
|||
doAnswer(new Answer<Void>() {
|
||||
@Override
|
||||
public Void answer(InvocationOnMock theInvocation) throws Throwable {
|
||||
IBaseResource res = (IBaseResource) theInvocation.getArguments()[0];
|
||||
IBaseResource res = (IBaseResource) theInvocation.getArguments()[1];
|
||||
assertEquals("Patient/" + id + "/_history/2", res.getIdElement().getValue());
|
||||
return null;
|
||||
}}).when(myRequestOperationCallback).resourceUpdated(any(IBaseResource.class));
|
||||
}}).when(myRequestOperationCallback).resourceUpdated(any(IBaseResource.class), any(IBaseResource.class));
|
||||
|
||||
p = new Patient();
|
||||
p.setId(new IdDt("Patient/" + id));
|
||||
|
@ -410,6 +411,7 @@ public class FhirResourceDaoDstu2InterceptorTest extends BaseJpaDstu2Test {
|
|||
assertEquals(2L, newId.getVersionIdPartAsLong().longValue());
|
||||
|
||||
verify(myRequestOperationCallback, times(1)).resourceUpdated(any(IBaseResource.class));
|
||||
verify(myRequestOperationCallback, times(1)).resourceUpdated(any(IBaseResource.class), any(IBaseResource.class));
|
||||
verify(myRequestOperationCallback, times(1)).resourceCreated(any(IBaseResource.class));
|
||||
verifyNoMoreInteractions(myRequestOperationCallback);
|
||||
}
|
||||
|
|
|
@ -226,10 +226,12 @@ public class FhirResourceDaoDstu3InterceptorTest extends BaseJpaDstu3Test {
|
|||
verify(myServerOperationInterceptor, times(1)).resourceCreated(Mockito.isNull(RequestDetails.class), any(IBaseResource.class));
|
||||
}
|
||||
|
||||
@SuppressWarnings("deprecation")
|
||||
@Test
|
||||
public void testServerOperationUpdate() {
|
||||
verify(myServerOperationInterceptor, times(0)).resourceCreated(Mockito.isNull(RequestDetails.class), any(IBaseResource.class));
|
||||
verify(myServerOperationInterceptor, times(0)).resourceUpdated(Mockito.isNull(RequestDetails.class), any(IBaseResource.class));
|
||||
verify(myServerOperationInterceptor, times(0)).resourceUpdated(Mockito.isNull(RequestDetails.class), any(IBaseResource.class), any(IBaseResource.class));
|
||||
|
||||
Patient p = new Patient();
|
||||
p.addName().setFamily("PATIENT");
|
||||
|
@ -241,6 +243,7 @@ public class FhirResourceDaoDstu3InterceptorTest extends BaseJpaDstu3Test {
|
|||
|
||||
verify(myServerOperationInterceptor, times(1)).resourceCreated(Mockito.isNull(RequestDetails.class), any(IBaseResource.class));
|
||||
verify(myServerOperationInterceptor, times(1)).resourceUpdated(Mockito.isNull(RequestDetails.class), any(IBaseResource.class));
|
||||
verify(myServerOperationInterceptor, times(1)).resourceUpdated(Mockito.isNull(RequestDetails.class), any(IBaseResource.class), any(IBaseResource.class));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -429,10 +432,10 @@ public class FhirResourceDaoDstu3InterceptorTest extends BaseJpaDstu3Test {
|
|||
doAnswer(new Answer<Void>() {
|
||||
@Override
|
||||
public Void answer(InvocationOnMock theInvocation) throws Throwable {
|
||||
IBaseResource res = (IBaseResource) theInvocation.getArguments()[0];
|
||||
IBaseResource res = (IBaseResource) theInvocation.getArguments()[1];
|
||||
assertEquals("Patient/" + id + "/_history/2", res.getIdElement().getValue());
|
||||
return null;
|
||||
}}).when(myRequestOperationCallback).resourceUpdated(any(IBaseResource.class));
|
||||
}}).when(myRequestOperationCallback).resourceUpdated(any(IBaseResource.class), any(IBaseResource.class));
|
||||
|
||||
Bundle xactBundle = new Bundle();
|
||||
xactBundle.setType(BundleType.TRANSACTION);
|
||||
|
@ -448,6 +451,7 @@ public class FhirResourceDaoDstu3InterceptorTest extends BaseJpaDstu3Test {
|
|||
assertEquals(2L, newId.getVersionIdPartAsLong().longValue());
|
||||
|
||||
verify(myRequestOperationCallback, times(1)).resourceUpdated(any(IBaseResource.class));
|
||||
verify(myRequestOperationCallback, times(1)).resourceUpdated(any(IBaseResource.class), any(IBaseResource.class));
|
||||
verify(myRequestOperationCallback, times(1)).resourceCreated(any(IBaseResource.class));
|
||||
verifyNoMoreInteractions(myRequestOperationCallback);
|
||||
}
|
||||
|
@ -462,9 +466,11 @@ public class FhirResourceDaoDstu3InterceptorTest extends BaseJpaDstu3Test {
|
|||
@Override
|
||||
public Void answer(InvocationOnMock theInvocation) throws Throwable {
|
||||
IBaseResource res = (IBaseResource) theInvocation.getArguments()[0];
|
||||
assertEquals("Patient/" + id + "/_history/1", res.getIdElement().getValue());
|
||||
res = (IBaseResource) theInvocation.getArguments()[1];
|
||||
assertEquals("Patient/" + id + "/_history/2", res.getIdElement().getValue());
|
||||
return null;
|
||||
}}).when(myRequestOperationCallback).resourceUpdated(any(IBaseResource.class));
|
||||
}}).when(myRequestOperationCallback).resourceUpdated(any(IBaseResource.class), any(IBaseResource.class));
|
||||
|
||||
p = new Patient();
|
||||
p.setId(new IdType("Patient/" + id));
|
||||
|
@ -473,6 +479,7 @@ public class FhirResourceDaoDstu3InterceptorTest extends BaseJpaDstu3Test {
|
|||
assertEquals(2L, newId.getVersionIdPartAsLong().longValue());
|
||||
|
||||
verify(myRequestOperationCallback, times(1)).resourceUpdated(any(IBaseResource.class));
|
||||
verify(myRequestOperationCallback, times(1)).resourceUpdated(any(IBaseResource.class), any(IBaseResource.class));
|
||||
verify(myRequestOperationCallback, times(1)).resourceCreated(any(IBaseResource.class));
|
||||
verifyNoMoreInteractions(myRequestOperationCallback);
|
||||
}
|
||||
|
|
|
@ -14,10 +14,8 @@ import org.apache.http.client.methods.HttpDelete;
|
|||
import org.apache.http.client.methods.HttpPost;
|
||||
import org.apache.http.entity.ContentType;
|
||||
import org.apache.http.entity.StringEntity;
|
||||
import org.hl7.fhir.dstu3.model.IdType;
|
||||
import org.hl7.fhir.dstu3.model.Observation;
|
||||
import org.hl7.fhir.dstu3.model.*;
|
||||
import org.hl7.fhir.dstu3.model.Observation.ObservationStatus;
|
||||
import org.hl7.fhir.dstu3.model.Patient;
|
||||
import org.hl7.fhir.instance.model.api.IIdType;
|
||||
import org.junit.AfterClass;
|
||||
import org.junit.Test;
|
||||
|
@ -183,6 +181,63 @@ public class AuthorizationInterceptorResourceProviderDstu3Test extends BaseResou
|
|||
|
||||
}
|
||||
|
||||
/**
|
||||
* See #667
|
||||
*/
|
||||
@Test
|
||||
public void testBlockUpdatingPatientUserDoesnNotHaveAccessTo() throws IOException {
|
||||
Patient pt1 = new Patient();
|
||||
pt1.setActive(true);
|
||||
final IIdType pid1 = ourClient.create().resource(pt1).execute().getId().toUnqualifiedVersionless();
|
||||
|
||||
Patient pt2 = new Patient();
|
||||
pt2.setActive(false);
|
||||
final IIdType pid2 = ourClient.create().resource(pt2).execute().getId().toUnqualifiedVersionless();
|
||||
|
||||
ourRestServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {
|
||||
@Override
|
||||
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
|
||||
return new RuleBuilder()
|
||||
.allow().write().allResources().inCompartment("Patient", pid1).andThen()
|
||||
.build();
|
||||
}
|
||||
});
|
||||
|
||||
Observation obs = new Observation();
|
||||
obs.setStatus(ObservationStatus.FINAL);
|
||||
obs.setSubject(new Reference(pid1));
|
||||
IIdType oid = ourClient.create().resource(obs).execute().getId().toUnqualified();
|
||||
|
||||
|
||||
unregisterInterceptors();
|
||||
ourRestServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {
|
||||
@Override
|
||||
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
|
||||
return new RuleBuilder()
|
||||
.allow().write().allResources().inCompartment("Patient", pid2).andThen()
|
||||
.build();
|
||||
}
|
||||
});
|
||||
|
||||
/*
|
||||
* Try to update to a new patient. The user has access to write to things in
|
||||
* pid2's compartment, so this would normally be ok, but in this case they are overwriting
|
||||
* a resource that is already in pid1's compartment, which shouldn't be allowed.
|
||||
*/
|
||||
obs = new Observation();
|
||||
obs.setId(oid);
|
||||
obs.setStatus(ObservationStatus.FINAL);
|
||||
obs.setSubject(new Reference(pid2));
|
||||
|
||||
try {
|
||||
ourClient.update().resource(obs).execute();
|
||||
fail();
|
||||
} catch (ForbiddenOperationException e) {
|
||||
// good
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testDeleteResourceConditional() throws IOException {
|
||||
String methodName = "testDeleteResourceConditional";
|
||||
|
|
|
@ -69,12 +69,14 @@ public class InterceptorDstu3Test {
|
|||
ourServlet.setInterceptors(myInterceptor1, myInterceptor2);
|
||||
}
|
||||
|
||||
@SuppressWarnings("deprecation")
|
||||
@Test
|
||||
public void testServerOperationInterceptorAdapterMethods() {
|
||||
ServerOperationInterceptorAdapter i = new ServerOperationInterceptorAdapter();
|
||||
i.resourceCreated(null, null);
|
||||
i.resourceDeleted(null, null);
|
||||
i.resourceUpdated(null, null);
|
||||
i.resourceUpdated(null, null, null);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -172,7 +174,6 @@ public class InterceptorDstu3Test {
|
|||
return Patient.class;
|
||||
}
|
||||
|
||||
@SuppressWarnings("unused")
|
||||
@Validate()
|
||||
public MethodOutcome validate(@ResourceParam Patient theResource) {
|
||||
return new MethodOutcome();
|
||||
|
|
|
@ -6,6 +6,25 @@
|
|||
<title>HAPI FHIR Changelog</title>
|
||||
</properties>
|
||||
<body>
|
||||
<release version="2.6" date="TBD">
|
||||
<action type="fix" issue="667">
|
||||
When using the AuthorizationInterceptor with the JPA server, when a client is updating a resource
|
||||
from A to B, the user now needs to have write permission for both A and B. This is particularly
|
||||
important for cases where (for example) an Observation is being updated from having a subject of
|
||||
Patient/A to Patient/B. If the user has write permission for Patient/B's compartment, this would
|
||||
previously have been allowed even if the user did not have access to write to Patient/A's compartment.
|
||||
Thanks to Eeva Turkka for reporting!
|
||||
</action>
|
||||
<action type="add">
|
||||
IServerOperationInterceptor now has a new method
|
||||
<![CDATA[<code>resourceUpdated(RequestDetails, IBaseResource, IBaseResource)</code>]]>
|
||||
which replaces the previous
|
||||
<![CDATA[<code>resourceUpdated(RequestDetails, IBaseResource)</code>]]>. This allows
|
||||
interceptors to be notified of resource updates, but also see what the resource
|
||||
looked like before the update. This change was made to support the change above, but
|
||||
seems like a useful feature all around.
|
||||
</action>
|
||||
</release>
|
||||
<release version="2.5" date="2017-06-08">
|
||||
<action type="fix">
|
||||
<![CDATA[
|
||||
|
|
Loading…
Reference in New Issue