Skip to content

Commit

Permalink
Call into $bulkdata-status passing in the encoded job number (#2129)
Browse files Browse the repository at this point in the history
Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
  • Loading branch information
prb112 authored Mar 23, 2021
1 parent 109c1ef commit f696c79
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 27 deletions.
2 changes: 1 addition & 1 deletion docs/src/pages/guides/FHIRServerUsersGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -2256,7 +2256,7 @@ must restart the server for that change to take effect.
|`fhirServer/bulkdata/core/cos/requestTimeout`|N|N|
|`fhirServer/bulkdata/core/cos/socketTimeout`|N|N|
|`fhirServer/bulkdata/core/cos/useServerTruststore`|Y|Y|
|`fhirServer/bulkdata/core/batchIdEncryptionKey`|N|N|
|`fhirServer/bulkdata/core/batchIdEncryptionKey`|Y|N|
|`fhirServer/bulkdata/core/pageSize`|Y|Y|
|`fhirServer/bulkdata/core/maxPartitions`|Y|Y|
|`fhirServer/bulkdata/core/maxInputs`|Y|Y|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ protected Parameters doInvoke(FHIROperationContext operationContext, Class<? ext
if (logicalId == null && versionId == null && resourceType == null) {
String method = (String) operationContext.getProperty(FHIROperationContext.PROPNAME_METHOD_TYPE);
if ("DELETE".equalsIgnoreCase(method)) {
// Assume GET or POST
String job = export.checkAndValidateJob(parameters);
// For now, we're going to execute the status update, and check.
// If Base, Export Status (Else Invalid)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@ public class BulkDataClient {

private static final HttpWrapper wrapper = new HttpWrapper();

private static final JobIdEncodingTransformer transformer = new JobIdEncodingTransformer();

private static final BulkDataExportUtil export = new BulkDataExportUtil();

// @formatter:off
Expand Down Expand Up @@ -270,7 +268,7 @@ public String submitExport(Instant since, List<String> types, ExportType exportT
}
cli.close();

return baseUri + "/$bulkdata-status?job=" + transformer.endcodeJobId(jobId);
return baseUri + "/$bulkdata-status?job=" + JobIdEncodingTransformer.getInstance().encodeJobId(jobId);
}

/**
Expand Down Expand Up @@ -829,7 +827,7 @@ public String submitImport(String inputFormat, String inputSource, List<Input> i
}
cli.close();

return baseUri + "/$bulkdata-status?job=" + transformer.endcodeJobId(jobId);
return baseUri + "/$bulkdata-status?job=" + JobIdEncodingTransformer.getInstance().encodeJobId(jobId);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ public abstract class AbstractSystemConfigurationImpl implements ConfigurationAd
private static final int coreFileResourceCountThreshold = defaultCoreFileResourceCountThreshold();
private static final int coreFileWriteTriggerSize = defaultCoreFileWriteTriggerSize();
private static final long coreFileSizeThreshold = defaultCoreFileSizeThreshold();
private static final String coreBatchIdEncryptionKey = defaultCoreBatchIdEncryptionKey();
private static final int coreMaxParititions = defaultCoreMaxParititions();
private static final int inputLimits = defaultInputLimits();

Expand All @@ -95,10 +94,6 @@ private static final int defaultCoreMaxParititions() {

@Override
public String getCoreBatchIdEncryptionKey() {
return coreBatchIdEncryptionKey;
}

private static final String defaultCoreBatchIdEncryptionKey() {
return FHIRConfigHelper.getStringProperty("fhirServer/bulkdata/core/batchIdEncryptionKey", null);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@
import java.security.MessageDigest;
import java.util.Arrays;
import java.util.Base64;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.logging.Level;
import java.util.logging.Logger;

import javax.crypto.Cipher;
import javax.crypto.spec.SecretKeySpec;

import com.ibm.fhir.config.FHIRRequestContext;
import com.ibm.fhir.operation.bulkdata.config.ConfigurationFactory;

/**
Expand All @@ -26,18 +29,32 @@ public class JobIdEncodingTransformer {
private static final String CLASSNAME = JobIdEncodingTransformer.class.getName();
private static final Logger logger = Logger.getLogger(CLASSNAME);

// Encryption key used for JavaBatch Job ID
private static final SecretKeySpec BATCHJOBID_ENCRYPTION_KEY = getJobIdEncryptionKey();
// Tenant Encryption key used for JavaBatch Job ID
private static ConcurrentMap<String, SecretKeySpec> KEY_MAP = new ConcurrentHashMap<>();

public JobIdEncodingTransformer() {
private static JobIdEncodingTransformer transformer = null;

private JobIdEncodingTransformer() {
// No Operation
}

/**
* get the instance
*
* @return
*/
public static JobIdEncodingTransformer getInstance() {
if (transformer == null) {
transformer = new JobIdEncodingTransformer();
}
return transformer;
}

/*
* gets the server-wide specific encryption key.
* gets the tenant specific encryption key.
* @return
*/
private static SecretKeySpec getJobIdEncryptionKey() {
private SecretKeySpec getJobIdEncryptionKey() {
String encryptionKey = ConfigurationFactory.getInstance().getCoreBatchIdEncryptionKey();
SecretKeySpec secretKey = null;

Expand Down Expand Up @@ -65,15 +82,17 @@ private static SecretKeySpec getJobIdEncryptionKey() {
* @param jobId
* @return
*/
public String endcodeJobId(String jobId) {
public String encodeJobId(String jobId) {
String tenantId = FHIRRequestContext.get().getTenantId();
SecretKeySpec key = KEY_MAP.computeIfAbsent(tenantId, k -> getJobIdEncryptionKey());
// Encrypt and UrlEncode the batch job id.
if (BATCHJOBID_ENCRYPTION_KEY == null) {
if (key == null) {
return jobId;
} else {
try {
// Use light weight encryption without salt to simplify both the encryption/decryption and also config.
Cipher cp = Cipher.getInstance("AES/ECB/PKCS5Padding");
cp.init(Cipher.ENCRYPT_MODE, BATCHJOBID_ENCRYPTION_KEY);
cp.init(Cipher.ENCRYPT_MODE, key);

// Encrypt the job id, base64-encode it, and replace all `/` chars with the less problematic `_` char
String encodedJobId = Base64.getEncoder().withoutPadding().encodeToString(cp.doFinal(jobId.getBytes("UTF-8"))).replaceAll("/", "_");
Expand All @@ -88,20 +107,23 @@ public String endcodeJobId(String jobId) {
/**
* decodes the job id.
*
* @implNote note a Cipher is used here, however it is not used to encrypt sensitive information rather encode the jobId.
* @implNote note a Cipher is used here, however it is not used to encrypt sensitive information rather encode the
* jobId.
*
* @param encodedJobId
* @return
*/
public String decodeJobId(String encodedJobId) {
String tenantId = FHIRRequestContext.get().getTenantId();
SecretKeySpec key = KEY_MAP.computeIfAbsent(tenantId, k -> getJobIdEncryptionKey());
// Decrypt to get the batch job id.
if (BATCHJOBID_ENCRYPTION_KEY == null) {
if (key == null) {
return encodedJobId;
} else {
try {
// Use light weight encryption without salt to simplify both the encryption/decryption and also config.
Cipher cp = Cipher.getInstance("AES/ECB/PKCS5PADDING");
cp.init(Cipher.DECRYPT_MODE, BATCHJOBID_ENCRYPTION_KEY);
cp.init(Cipher.DECRYPT_MODE, key);
// The encrypted job id has already been urldecoded by liberty runtime before reaching this function,
// so, we don't do urldecode here.)
return new String(cp.doFinal(Base64.getDecoder().decode(encodedJobId.replaceAll("_", "/"))), "UTF-8");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@
* BulkData Util captures common methods
*/
public class BulkDataExportUtil {

private static JobIdEncodingTransformer transformer = new JobIdEncodingTransformer();
private static Set<String> RESOURCE_TYPES = ModelSupport.getResourceTypes(false).stream()
.map(m -> m.getSimpleName())
.collect(Collectors.toSet());
Expand Down Expand Up @@ -296,7 +294,7 @@ public String checkAndValidateJob(Parameters parameters) throws FHIROperationExc
for (Parameters.Parameter parameter : parameters.getParameter()) {
if (OperationConstants.PARAM_JOB.equals(parameter.getName().getValue())
&& parameter.getValue() != null && parameter.getValue().is(com.ibm.fhir.model.type.String.class)) {
String job = transformer.decodeJobId(parameter.getValue().as(com.ibm.fhir.model.type.String.class).getValue());
String job = JobIdEncodingTransformer.getInstance().decodeJobId(parameter.getValue().as(com.ibm.fhir.model.type.String.class).getValue());

// The job is never going to be empty or null as STRING is never empty at this point.
if (job.contains("/") || job.contains("?")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public void clearThreadLocal() {
@Test
public void testTransformerRoundTrip() throws Exception {
// Using the legacy implementation for the configuration the encode/decode uses change-password
final JobIdEncodingTransformer transformer = new JobIdEncodingTransformer();
final JobIdEncodingTransformer transformer = JobIdEncodingTransformer.getInstance();

String jobId = transformer.decodeJobId("1");
assertNotNull(jobId);
Expand All @@ -65,7 +65,7 @@ public void testTransformerRoundTrip() throws Exception {
for (int i = 0; i < 2000; i++) {
jobId = String.valueOf(i);

String encodedJobId = transformer.endcodeJobId(jobId);
String encodedJobId = transformer.encodeJobId(jobId);
assertNotNull(encodedJobId);
assertFalse(encodedJobId.equals(jobId));
assertFalse(encodedJobId.startsWith("/"));
Expand Down

0 comments on commit f696c79

Please sign in to comment.