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

fix 403 on updates when version submitted without rewrite header (#3716) 

* fix 403 on updates when version submitted without rewrite header

* change log

Co-authored-by: Ken Stevens <ken@smilecdr.com>
This commit is contained in:
Ken Stevens 2022-06-17 15:51:47 -04:00 committed by GitHub
parent a18c83ae13
commit 427bdf330e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 83 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3716-fix-history-rewrite.yaml Normal file
View File

@ -0,0 +1,5 @@
---
type: fix
issue: 3716
title: "Server was blocking updates with Forbidden 403 that included a resource version in the url without
the X-Rewrite-History header. This has been corrected."

2
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java
View File

@ -1712,7 +1712,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
Runnable onRollback = () -> theResource.getIdElement().setValue(id);
// Execute the update in a retryable transaction
if (myDaoConfig.isUpdateWithHistoryRewriteEnabled() && "true".equals(theRequest.getHeader(JpaConstants.HEADER_REWRITE_HISTORY))) {
if (myDaoConfig.isUpdateWithHistoryRewriteEnabled() && theRequest.isRewriteHistory()) {
return myTransactionService.execute(theRequest, theTransactionDetails, tx -> doUpdateWithHistoryRewrite(theResource, theRequest, theTransactionDetails), onRollback);
} else {
return myTransactionService.execute(theRequest, theTransactionDetails, tx -> doUpdate(theResource, theMatchUrl, thePerformIndexing, theForceUpdateVersion, theRequest, theTransactionDetails), onRollback);

14
hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4HistoryRewriteTest.java
View File

@ -1,7 +1,7 @@
package ca.uhn.fhir.jpa.dao.r4;
import ca.uhn.fhir.jpa.model.util.JpaConstants;
import ca.uhn.fhir.jpa.test.BaseJpaR4Test;
import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException;
@ -42,7 +42,7 @@ public class FhirResourceDaoR4HistoryRewriteTest extends BaseJpaR4Test {
@AfterEach
public void tearDown() {
myDaoConfig.setUpdateWithHistoryRewriteEnabled(false);
when(mySrd.getHeader(eq(JpaConstants.HEADER_REWRITE_HISTORY))).thenReturn("");
when(mySrd.getHeader(eq(Constants.HEADER_REWRITE_HISTORY))).thenReturn("");
}
@Test
@ -54,7 +54,7 @@ public class FhirResourceDaoR4HistoryRewriteTest extends BaseJpaR4Test {
IIdType id = createPatientWithHistory();
// execute updates
when(mySrd.getHeader(eq(JpaConstants.HEADER_REWRITE_HISTORY))).thenReturn("true");
when(mySrd.isRewriteHistory()).thenReturn(true);
Patient p = new Patient();
p.addIdentifier().setSystem("urn:system").setValue(TEST_SYSTEM_NAME);
@ -100,7 +100,7 @@ public class FhirResourceDaoR4HistoryRewriteTest extends BaseJpaR4Test {
int resourceVersionsSizeInit = myResourceHistoryTableDao.findAll().size();
// execute update
when(mySrd.getHeader(eq(JpaConstants.HEADER_REWRITE_HISTORY))).thenReturn("true");
when(mySrd.isRewriteHistory()).thenReturn(true);
Patient p = new Patient();
p.addIdentifier().setSystem("urn:system").setValue(TEST_SYSTEM_NAME);
@ -151,7 +151,7 @@ public class FhirResourceDaoR4HistoryRewriteTest extends BaseJpaR4Test {
IIdType id = createPatientWithHistory();
// execute update
when(mySrd.getHeader(eq(JpaConstants.HEADER_REWRITE_HISTORY))).thenReturn("true");
when(mySrd.isRewriteHistory()).thenReturn(true);
Patient p = new Patient();
p.addIdentifier().setSystem("urn:system").setValue(TEST_SYSTEM_NAME);
@ -174,7 +174,7 @@ public class FhirResourceDaoR4HistoryRewriteTest extends BaseJpaR4Test {
IIdType id = createPatientWithHistory();
// execute update
when(mySrd.getHeader(eq(JpaConstants.HEADER_REWRITE_HISTORY))).thenReturn("true");
when(mySrd.isRewriteHistory()).thenReturn(true);
Patient p = new Patient();
p.addIdentifier().setSystem("urn:system").setValue(TEST_SYSTEM_NAME);
@ -197,7 +197,7 @@ public class FhirResourceDaoR4HistoryRewriteTest extends BaseJpaR4Test {
IIdType id = createPatientWithHistory();
// execute update
when(mySrd.getHeader(eq(JpaConstants.HEADER_REWRITE_HISTORY))).thenReturn("true");
when(mySrd.isRewriteHistory()).thenReturn(true);
Patient p = new Patient();
p.addIdentifier().setSystem("urn:system").setValue(TEST_SYSTEM_NAME);

8
hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/RequestDetails.java
View File

@ -76,6 +76,7 @@ public abstract class RequestDetails {
private String myRequestId;
private String myTransactionGuid;
private String myFixedConditionalUrl;
private boolean myRewriteHistory;
/**
* Constructor
@ -534,4 +535,11 @@ public abstract class RequestDetails {
}
public boolean isRewriteHistory() {
return myRewriteHistory;
}
public void setRewriteHistory(boolean theRewriteHistory) {
myRewriteHistory = theRewriteHistory;
}
}

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

@ -175,7 +175,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
if (theInputResource == null && theInputResourceId == null) {
return null;
}
if (theRequestDetails.getId() != null && theRequestDetails.getId().hasVersionIdPart() && theOperation == RestOperationTypeEnum.UPDATE) {
if (theRequestDetails.isRewriteHistory() && theRequestDetails.getId() != null && theRequestDetails.getId().hasVersionIdPart() && theOperation == RestOperationTypeEnum.UPDATE) {
return null;
}
switch (theOperation) {

22
hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/servlet/ServletRequestDetails.java
View File

@ -20,8 +20,8 @@ package ca.uhn.fhir.rest.server.servlet;
* #L%
*/
import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster;
import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.server.RequestDetails;
@ -30,6 +30,7 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.Validate;
import javax.annotation.Nonnull;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.ByteArrayInputStream;
@ -37,7 +38,12 @@ import java.io.IOException;
import java.io.InputStream;
import java.io.Reader;
import java.nio.charset.Charset;
import java.util.*;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.zip.GZIPInputStream;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
@ -54,7 +60,7 @@ public class ServletRequestDetails extends RequestDetails {
* Constructor for testing only
*/
public ServletRequestDetails() {
this((IInterceptorBroadcaster)null);
this((IInterceptorBroadcaster) null);
}
/**
@ -171,8 +177,13 @@ public class ServletRequestDetails extends RequestDetails {
this.myServer = theServer;
}
public ServletRequestDetails setServletRequest(HttpServletRequest myServletRequest) {
public ServletRequestDetails setServletRequest(@Nonnull HttpServletRequest myServletRequest) {
this.myServletRequest = myServletRequest;
// TODO KHS move a bunch of other initialization from RestfulServer into this method
if ("true".equals(myServletRequest.getHeader(Constants.HEADER_REWRITE_HISTORY))) {
setRewriteHistory(true);
}
return this;
}
@ -180,7 +191,7 @@ public class ServletRequestDetails extends RequestDetails {
this.myServletResponse = myServletResponse;
}
public Map<String,List<String>> getHeaders() {
public Map<String, List<String>> getHeaders() {
Map<String, List<String>> retVal = new HashMap<>();
Enumeration<String> names = myServletRequest.getHeaderNames();
while (names.hasMoreElements()) {
@ -194,4 +205,5 @@ public class ServletRequestDetails extends RequestDetails {
}
return Collections.unmodifiableMap(retVal);
}
}

44
hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/servlet/ServletRequestDetailsTest.java Normal file
View File

@ -0,0 +1,44 @@
package ca.uhn.fhir.rest.server.servlet;
import ca.uhn.fhir.rest.api.Constants;
import org.junit.jupiter.api.Test;
import javax.servlet.http.HttpServletRequest;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
class ServletRequestDetailsTest {
@Test
public void testRewriteHistoryHeader() {
ServletRequestDetails servletRequestDetails = new ServletRequestDetails();
HttpServletRequest httpRequest = mock(HttpServletRequest.class);
servletRequestDetails.setServletRequest(httpRequest);
when(httpRequest.getHeader(Constants.HEADER_REWRITE_HISTORY)).thenReturn("true");
servletRequestDetails.setServletRequest(httpRequest);
assertTrue(servletRequestDetails.isRewriteHistory());
}
@Test
public void testRewriteHistoryHeaderNull() {
ServletRequestDetails servletRequestDetails = new ServletRequestDetails();
HttpServletRequest httpRequest = mock(HttpServletRequest.class);
servletRequestDetails.setServletRequest(httpRequest);
when(httpRequest.getHeader(Constants.HEADER_REWRITE_HISTORY)).thenReturn(null);
servletRequestDetails.setServletRequest(httpRequest);
assertFalse(servletRequestDetails.isRewriteHistory());
}
@Test
public void testRewriteHistoryHeaderFalse() {
ServletRequestDetails servletRequestDetails = new ServletRequestDetails();
HttpServletRequest httpRequest = mock(HttpServletRequest.class);
servletRequestDetails.setServletRequest(httpRequest);
when(httpRequest.getHeader(Constants.HEADER_REWRITE_HISTORY)).thenReturn("false");
servletRequestDetails.setServletRequest(httpRequest);
assertFalse(servletRequestDetails.isRewriteHistory());
}
}