From e7d4257b87f8011dfb4a5d6ef3166e6b274a3a07 Mon Sep 17 00:00:00 2001 From: Chris Knoll Date: Mon, 13 Apr 2026 12:13:49 -0400 Subject: [PATCH 1/2] Removed ActivityTracker. Cleaned up permission annotations to use isAnyPermitted(anyOf) to avoid naming conflicts. --- .../org/ohdsi/webapi/activity/Activity.java | 34 ---------- .../org/ohdsi/webapi/activity/Tracker.java | 64 ------------------- .../cohortcharacterization/CcController.java | 20 +++--- .../CohortDefinitionService.java | 18 ++++-- .../feanalysis/FeAnalysisController.java | 2 +- .../webapi/ircalc/IRAnalysisService.java | 16 ++--- .../webapi/pathway/PathwayController.java | 12 ++-- .../webapi/security/authc/LoginService.java | 3 +- .../authz/AuthorizationCacheService.java | 5 ++ .../webapi/security/authz/RoleService.java | 15 ++++- .../webapi/security/authz/UserController.java | 12 ---- .../spring/WebApiSecurityExpressionRoot.java | 2 +- .../webapi/service/VocabularyService.java | 4 -- 13 files changed, 56 insertions(+), 151 deletions(-) delete mode 100644 src/main/java/org/ohdsi/webapi/activity/Activity.java delete mode 100644 src/main/java/org/ohdsi/webapi/activity/Tracker.java diff --git a/src/main/java/org/ohdsi/webapi/activity/Activity.java b/src/main/java/org/ohdsi/webapi/activity/Activity.java deleted file mode 100644 index f8c74524d7..0000000000 --- a/src/main/java/org/ohdsi/webapi/activity/Activity.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright 2015 fdefalco. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * 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 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.ohdsi.webapi.activity; - -import java.util.Date; - -/** - * - * @author fdefalco - */ -public class Activity { - - public enum ActivityType { - - Search, Import - }; - - public ActivityType type; - public String caption; - public Date timestamp; -} diff --git a/src/main/java/org/ohdsi/webapi/activity/Tracker.java b/src/main/java/org/ohdsi/webapi/activity/Tracker.java deleted file mode 100644 index 9cf55c1dec..0000000000 --- a/src/main/java/org/ohdsi/webapi/activity/Tracker.java +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright 2015 fdefalco. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * 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 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.ohdsi.webapi.activity; - -import java.util.Date; -import java.util.concurrent.ConcurrentLinkedQueue; - -import org.ohdsi.webapi.activity.Activity.ActivityType; -import org.springframework.http.MediaType; -import org.springframework.web.bind.annotation.GetMapping; -import org.springframework.web.bind.annotation.RequestMapping; -import org.springframework.web.bind.annotation.RestController; - -/** - * Activity tracker service - * - * @author fdefalco - */ -@RestController -@RequestMapping("/activity") -@Deprecated -public class Tracker { - private static ConcurrentLinkedQueue activityLog; - - public static void trackActivity(ActivityType type, String caption) { - if (activityLog == null) { - activityLog = new ConcurrentLinkedQueue<>(); - } - - Activity activity = new Activity(); - activity.caption = caption; - activity.timestamp = new Date(); - activity.type = type; - - activityLog.add(activity); - } - - public static Object[] getActivity() { - if (activityLog == null) { - activityLog = new ConcurrentLinkedQueue<>(); - } - - return activityLog.toArray(); - } - - @GetMapping(value = "/latest", produces = MediaType.APPLICATION_JSON_VALUE) - @Deprecated - public Object[] getLatestActivity() { - return getActivity(); - } -} diff --git a/src/main/java/org/ohdsi/webapi/cohortcharacterization/CcController.java b/src/main/java/org/ohdsi/webapi/cohortcharacterization/CcController.java index ec76c0a4b6..47a9c36ee0 100644 --- a/src/main/java/org/ohdsi/webapi/cohortcharacterization/CcController.java +++ b/src/main/java/org/ohdsi/webapi/cohortcharacterization/CcController.java @@ -119,7 +119,7 @@ public CohortCharacterizationDTO create(@RequestBody final CohortCharacterizatio * @return The cohort characterization definition of the newly created copy */ @PostMapping(value = "/{id}", produces = MediaType.APPLICATION_JSON_VALUE) - @PreAuthorize("(isOwner(#id, COHORT_CHARACTERIZATION) or isPermitted(anyOf('read:cohort-characterization','write:cohort-characterization')) or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)) and isPermitted('create:cohort-characterization')") + @PreAuthorize("(isOwner(#id, COHORT_CHARACTERIZATION) or isAnyPermitted(anyOf('read:cohort-characterization','write:cohort-characterization')) or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)) and isPermitted('create:cohort-characterization')") public CohortCharacterizationDTO copy(@PathVariable("id") final Long id) { CohortCharacterizationDTO dto = getDesign(id); dto.setName(service.getNameForCopy(dto.getName())); @@ -162,7 +162,7 @@ public Page listDesign(@Pagination Pageable pageable) * @return name, createdDate, tags, etc for a single cohort characterization. */ @GetMapping(value = "/{id}", produces = MediaType.APPLICATION_JSON_VALUE) - @PreAuthorize("isOwner(#id, COHORT_CHARACTERIZATION) or isPermitted('read:cohort-characterization','write:cohort-characterization') or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)") + @PreAuthorize("isOwner(#id, COHORT_CHARACTERIZATION) or isAnyPermitted(anyOf('read:cohort-characterization','write:cohort-characterization')) or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)") public CcShortDTO get(@PathVariable("id") final Long id) { return convertCcToShortDto(service.findById(id)); } @@ -174,7 +174,7 @@ public CcShortDTO get(@PathVariable("id") final Long id) { * @return JSON containing the cohort characterization specification */ @GetMapping(value = "/{id}/design", produces = MediaType.APPLICATION_JSON_VALUE) - @PreAuthorize("isOwner(#id, COHORT_CHARACTERIZATION) or isPermitted('read:cohort-characterization','write:cohort-characterization') or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)") + @PreAuthorize("isOwner(#id, COHORT_CHARACTERIZATION) or isAnyPermitted(anyOf('read:cohort-characterization','write:cohort-characterization')) or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)") public CohortCharacterizationDTO getDesign(@PathVariable("id") final Long id) { CohortCharacterizationEntity cc = service.findByIdWithLinkedEntities(id); ExceptionUtils.throwNotFoundExceptionIfNull(cc, String.format("There is no cohort characterization with id = %d.", id)); @@ -248,7 +248,7 @@ public CohortCharacterizationDTO doImport(@RequestBody final CcExportDTO dto) { * @return JSON containing the cohort characterization definition */ @GetMapping(value = "/{id}/export", produces = MediaType.APPLICATION_JSON_VALUE) - @PreAuthorize("isOwner(#id, COHORT_CHARACTERIZATION) or isPermitted('read:cohort-characterization','write:cohort-characterization') or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)") + @PreAuthorize("isOwner(#id, COHORT_CHARACTERIZATION) or isAnyPermitted(anyOf('read:cohort-characterization','write:cohort-characterization')) or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)") public String export(@PathVariable("id") final Long id) { return service.serializeCc(id); } @@ -259,7 +259,7 @@ public String export(@PathVariable("id") final Long id) { * @return A zip file containing three csv files (mappedConcepts, includedConcepts, conceptSetExpression) */ @GetMapping(value = "/{id}/export/conceptset", produces = MediaType.APPLICATION_OCTET_STREAM_VALUE) - @PreAuthorize("isOwner(#id, COHORT_CHARACTERIZATION) or isPermitted('read:cohort-characterization') or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)") + @PreAuthorize("isOwner(#id, COHORT_CHARACTERIZATION) or isAnyPermitted('read:cohort-characterization','write:cohort-characterization') or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)") public ResponseEntity exportConceptSets(@PathVariable("id") final Long id) { CohortCharacterizationEntity cc = service.findById(id); @@ -287,7 +287,7 @@ public CheckResult runDiagnostics(@RequestBody CohortCharacterizationDTO charact * @return A json object with information about the generation job included the status and execution id. */ @PostMapping(value = "/{id}/generation/{sourceKey}", produces = MediaType.APPLICATION_JSON_VALUE) - @PreAuthorize("(isOwner(#id, COHORT_CHARACTERIZATION) or isPermitted(anyOf('write:cohort-characterization','read:cohort-characterization')) or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)) and (isPermitted('write:source') or hasSourceAccess(#sourceKey, WRITE))") + @PreAuthorize("(isOwner(#id, COHORT_CHARACTERIZATION) or isAnyPermitted(anyOf('write:cohort-characterization','read:cohort-characterization')) or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)) and (isPermitted('write:source') or hasSourceAccess(#sourceKey, WRITE))") public JobExecutionResource generate(@PathVariable("id") final Long id, @PathVariable("sourceKey") final String sourceKey) { CohortCharacterizationEntity cc = service.findByIdWithLinkedEntities(id); ExceptionUtils.throwNotFoundExceptionIfNull(cc, String.format("There is no cohort characterization with id = %d.", id)); @@ -305,7 +305,7 @@ public JobExecutionResource generate(@PathVariable("id") final Long id, @PathVar * @return Status code */ @DeleteMapping(value = "/{id}/generation/{sourceKey}") - @PreAuthorize("(isOwner(#id, COHORT_CHARACTERIZATION) or isPermitted(anyOf('write:cohort-characterization','read:cohort-characterization')) or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)) and (isPermitted('write:source') or hasSourceAccess(#sourceKey, WRITE))") + @PreAuthorize("(isOwner(#id, COHORT_CHARACTERIZATION) or isAnyPermitted(anyOf('write:cohort-characterization','read:cohort-characterization')) or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)) and (isPermitted('write:source') or hasSourceAccess(#sourceKey, WRITE))") public ResponseEntity cancelGeneration(@PathVariable("id") final Long id, @PathVariable("sourceKey") final String sourceKey) { service.cancelGeneration(id, sourceKey); return ResponseEntity.ok().build(); @@ -317,7 +317,7 @@ public ResponseEntity cancelGeneration(@PathVariable("id") final Long id, * @return An array of all generations that includes the generation id, sourceKey, start and end times */ @GetMapping(value = "/{id}/generation", produces = MediaType.APPLICATION_JSON_VALUE) - @PreAuthorize("isOwner(#id, COHORT_CHARACTERIZATION) or isPermitted(anyOf('read:cohort-characterization','write:cohort-characterization')) or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)") + @PreAuthorize("isOwner(#id, COHORT_CHARACTERIZATION) or isAnyPermitted(anyOf('read:cohort-characterization','write:cohort-characterization')) or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)") public List getGenerationList(@PathVariable("id") final Long id) { Map sourcesMap = sourceService.getSourcesMap(SourceMapKey.BY_SOURCE_KEY); @@ -525,7 +525,7 @@ public void unassignPermissionProtectedTag(@PathVariable("id") final long id, @P * @return */ @GetMapping(value = "/{id}/version", produces = MediaType.APPLICATION_JSON_VALUE) - @PreAuthorize("isOwner(#id, COHORT_CHARACTERIZATION) or isPermitted(anyOf('read:cohort-characterization','write:cohort-characterization')) or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)") + @PreAuthorize("isOwner(#id, COHORT_CHARACTERIZATION) or isAnyPermitted(anyOf('read:cohort-characterization','write:cohort-characterization')) or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)") public List getVersions(@PathVariable("id") final long id) { return service.getVersions(id); } @@ -538,7 +538,7 @@ public List getVersions(@PathVariable("id") final long id) { * @return */ @GetMapping(value = "/{id}/version/{version}", produces = MediaType.APPLICATION_JSON_VALUE) - @PreAuthorize("isOwner(#id, COHORT_CHARACTERIZATION) or isPermitted(anyOf('read:cohort-characterization','write:cohort-characterization')) or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)") + @PreAuthorize("isOwner(#id, COHORT_CHARACTERIZATION) or isAnyPermitted(anyOf('read:cohort-characterization','write:cohort-characterization')) or hasEntityAccess(#id, COHORT_CHARACTERIZATION, READ)") public CcVersionFullDTO getVersion(@PathVariable("id") final long id, @PathVariable("version") final int version) { return service.getVersion(id, version); } diff --git a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortDefinitionService.java b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortDefinitionService.java index e637e8ad2d..e545d21cd5 100644 --- a/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortDefinitionService.java +++ b/src/main/java/org/ohdsi/webapi/cohortdefinition/CohortDefinitionService.java @@ -134,6 +134,7 @@ import org.springframework.stereotype.Component; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.TransactionStatus; +import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.support.TransactionCallbackWithoutResult; import org.springframework.transaction.support.TransactionTemplate; @@ -524,7 +525,7 @@ public CohortDTO createCohortDefinition(@RequestBody CohortDTO dto) { * @return The cohort definition JSON expression */ @GetMapping(value = "/{id}", produces = MediaType.APPLICATION_JSON_VALUE) - @PreAuthorize("isOwner(#id, COHORT_DEFINITION) or isPermitted('read:cohort-definition') or isPermitted('write:cohort-definition') or hasEntityAccess(#id, COHORT_DEFINITION, READ)") + @PreAuthorize("isOwner(#id, COHORT_DEFINITION) or isAnyPermitted(anyOf('read:cohort-definition','write:cohort-definition')) or hasEntityAccess(#id, COHORT_DEFINITION, READ)") public CohortRawDTO getCohortDefinitionRaw(@PathVariable("id") final int id) { return getTransactionTemplate().execute(transactionStatus -> { CohortDefinitionEntity d = this.cohortDefinitionRepository.findOneWithDetail(id); @@ -613,7 +614,7 @@ public CohortDTO saveCohortDefinition(@PathVariable("id") final int id, @Request */ @GetMapping(value = "/{id}/generate/{sourceKey}", produces = MediaType.APPLICATION_JSON_VALUE) @PreAuthorize(""" - (isOwner(#id, COHORT_DEFINITION) or isPermitted(anyOf('write:cohort-definition','read:cohort-definition')) or hasEntityAccess(#id, COHORT_DEFINITION, READ)) + (isOwner(#id, COHORT_DEFINITION) or isAnyPermitted(anyOf('write:cohort-definition','read:cohort-definition')) or hasEntityAccess(#id, COHORT_DEFINITION, READ)) and (isPermitted('write:source') or hasSourceAccess(#sourceKey, WRITE)) """) public JobExecutionResource generateCohort(@PathVariable("id") final int id, @@ -771,6 +772,7 @@ public List getNamesLike(String copyName) { @DeleteMapping(value = "/{id}", produces = MediaType.APPLICATION_JSON_VALUE) @CacheEvict(cacheNames = CachingSetup.COHORT_DEFINITION_LIST_CACHE, allEntries = true) @PreAuthorize("isOwner(#id, COHORT_DEFINITION) or isPermitted('write:cohort-definition') or hasEntityAccess(#id, COHORT_DEFINITION, WRITE)") + @Transactional(propagation = Propagation.NOT_SUPPORTED) public void delete(@PathVariable("id") final int id) { // perform the JPA update in a separate transaction this.getTransactionTemplateRequiresNew().execute(new TransactionCallbackWithoutResult() { @@ -785,18 +787,20 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { JobParameters parameters = e.getJobParameters(); String jobName = e.getJobInstance().getJobName(); return Objects.equals(parameters.getString(COHORT_DEFINITION_ID), Integer.toString(id)) - && Objects.equals(parameters.getString(SOURCE_ID), Integer.toString(sourceId)) - && Objects.equals(Constants.GENERATE_COHORT, jobName); + && Objects.equals(parameters.getString(SOURCE_ID), Integer.toString(sourceId)) + && Objects.equals(Constants.GENERATE_COHORT, jobName); }); }); cohortDefinitionRepository.delete(def); - samplingService.launchDeleteSamplesTasklet(id); } else { log.warn("Failed to delete Cohort Definition with ID = {}", id); } } }); + // Launch cleanup job outside of transaction to avoid conflict with batch transaction manager + samplingService.launchDeleteSamplesTasklet(id); + JobParametersBuilder builder = new JobParametersBuilder(); builder.addString(JOB_NAME, String.format("Cleanup cohort %d.", id)); builder.addString(COHORT_DEFINITION_ID, ("" + id)); @@ -887,8 +891,8 @@ public ResponseEntity getAnalysisInfo(final int id) { } @Override - @PreAuthorize("(isOwner(#id, INCIDENCE_RATE) or isPermitted('read:incidence') or isPermitted('write:incidence') or hasEntityAccess(#id, INCIDENCE_RATE, READ)) and (isPermitted(anyOf('read:source','write:source')) or hasSourceAccess(#sourceKey, READ))") + @PreAuthorize("(isOwner(#id, INCIDENCE_RATE) or isAnyPermitted(anyOf('read:incidence','write:incidence')) or hasEntityAccess(#id, INCIDENCE_RATE, READ)) and (isAnyPermitted(anyOf('read:source','write:source')) or hasSourceAccess(#sourceKey, READ))") @Transactional(readOnly = true) public AnalysisInfoDTO getAnalysisInfo(int id, String sourceKey) { @@ -587,7 +587,7 @@ public AnalysisInfoDTO getAnalysisInfo(int id, String sourceKey) { } @Override - @PreAuthorize("(isOwner(#id, INCIDENCE_RATE) or isPermitted('read:incidence') or isPermitted('write:incidence') or hasEntityAccess(#id, INCIDENCE_RATE, READ)) and (isPermitted(anyOf('read:source','write:source')) or hasSourceAccess(#sourceKey, READ))") + @PreAuthorize("(isOwner(#id, INCIDENCE_RATE) or isPermitted('read:incidence') or isAnyPermitted('write:incidence') or hasEntityAccess(#id, INCIDENCE_RATE, READ)) and (isAnyPermitted(anyOf('read:source','write:source')) or hasSourceAccess(#sourceKey, READ))") @Transactional public AnalysisReport getAnalysisReport(final int id, final String sourceKey, final int targetId, final int outcomeId ) { @@ -628,7 +628,7 @@ public CheckResult runDiagnostics(IRAnalysisDTO irAnalysisDTO){ @Override @Transactional - @PreAuthorize("(isOwner(#id, INCIDENCE_RATE) or isPermitted(anyOf('read:incidence','write:incidence')) or hasEntityAccess(#id, INCIDENCE_RATE, READ)) and isPermitted('create:incidence')") + @PreAuthorize("(isOwner(#id, INCIDENCE_RATE) or isAnyPermitted(anyOf('read:incidence','write:incidence')) or hasEntityAccess(#id, INCIDENCE_RATE, READ)) and isPermitted('create:incidence')") public IRAnalysisDTO copy(final int id) { IRAnalysisDTO analysis = getAnalysis(id); analysis.setTags(null); @@ -640,7 +640,7 @@ public IRAnalysisDTO copy(final int id) { @Override @Transactional - @PreAuthorize("isOwner(#id, INCIDENCE_RATE) or isPermitted('read:incidence') or isPermitted('write:incidence') or hasEntityAccess(#id, INCIDENCE_RATE, READ)") + @PreAuthorize("isOwner(#id, INCIDENCE_RATE) or isAnyPermitted(anyOf('read:incidence','write:incidence')) or hasEntityAccess(#id, INCIDENCE_RATE, READ)") public ResponseEntity export(final int id) { Map fileList = new HashMap<>(); @@ -770,7 +770,7 @@ public void delete(final int id) { @Override @Transactional - @PreAuthorize("(isOwner(#id, INCIDENCE_RATE) or isPermitted(anyOf('read:incidence','write:incidence')) or hasEntityAccess(#id, INCIDENCE_RATE, READ)) and (isPermitted('write:source') or hasSourceAccess(#sourceKey, WRITE))") + @PreAuthorize("(isOwner(#id, INCIDENCE_RATE) or isAnyPermitted(anyOf('read:incidence','write:incidence')) or hasEntityAccess(#id, INCIDENCE_RATE, READ)) and (isPermitted('write:source') or hasSourceAccess(#sourceKey, WRITE))") public void deleteInfo(final int id, final String sourceKey) { IncidenceRateAnalysis analysis = irAnalysisRepository.findById(id).orElseThrow(); ExecutionInfo itemToRemove = null; @@ -855,7 +855,7 @@ public void deleteVersion(int id, int version) { } @Override - @PreAuthorize("(isOwner(#id, INCIDENCE_RATE) or isPermitted(anyOf('read:incidence','write:incidence')) or hasEntityAccess(#id, INCIDENCE_RATE, READ)) and isPermitted('create:incidence')") + @PreAuthorize("(isOwner(#id, INCIDENCE_RATE) or isAnyPermitted(anyOf('read:incidence','write:incidence')) or hasEntityAccess(#id, INCIDENCE_RATE, READ)) and isPermitted('create:incidence')") @Transactional public IRAnalysisDTO copyAssetFromVersion(int id, int version) { checkVersion(id, version, false); diff --git a/src/main/java/org/ohdsi/webapi/pathway/PathwayController.java b/src/main/java/org/ohdsi/webapi/pathway/PathwayController.java index 63b4a1434a..477d0a2df2 100644 --- a/src/main/java/org/ohdsi/webapi/pathway/PathwayController.java +++ b/src/main/java/org/ohdsi/webapi/pathway/PathwayController.java @@ -101,7 +101,7 @@ public PathwayAnalysisDTO create(@RequestBody final PathwayAnalysisDTO dto) { * @return The copied pathway analysis. */ @PostMapping(value = "/{id}", produces = MediaType.APPLICATION_JSON_VALUE) - @PreAuthorize("(isOwner(#id, PATHWAY_ANALYSIS) or isPermitted(anyOf('read:pathway','write:pathway')) or hasEntityAccess(#id, PATHWAY_ANALYSIS, READ)) and isPermitted('create:pathway')") + @PreAuthorize("(isOwner(#id, PATHWAY_ANALYSIS) or isAnyPermitted(anyOf('read:pathway','write:pathway')) or hasEntityAccess(#id, PATHWAY_ANALYSIS, READ)) and isPermitted('create:pathway')") public PathwayAnalysisDTO copy(@PathVariable("id") final Integer id) { PathwayAnalysisDTO dto = get(id); @@ -277,7 +277,7 @@ public void delete(@PathVariable("id") final Integer id) { */ @PostMapping(value = "/{id}/generation/{sourceKey}", produces = MediaType.APPLICATION_JSON_VALUE) @Transactional - @PreAuthorize("(isOwner(#pathwayAnalysisId, PATHWAY_ANALYSIS) or isPermitted(anyOf('read:pathway','write:pathway')) or hasEntityAccess(#pathwayAnalysisId, PATHWAY_ANALYSIS, READ)) and (isPermitted('write:source') or hasSourceAccess(#sourceKey, WRITE))") + @PreAuthorize("(isOwner(#pathwayAnalysisId, PATHWAY_ANALYSIS) or isAnyPermitted(anyOf('read:pathway','write:pathway')) or hasEntityAccess(#pathwayAnalysisId, PATHWAY_ANALYSIS, READ)) and (isPermitted('write:source') or hasSourceAccess(#sourceKey, WRITE))") public JobExecutionResource generatePathways( @PathVariable("id") final Integer pathwayAnalysisId, @PathVariable("sourceKey") final String sourceKey @@ -306,7 +306,7 @@ public JobExecutionResource generatePathways( * @param sourceKey the key of the source */ @DeleteMapping(value = "/{id}/generation/{sourceKey}") - @PreAuthorize("(isOwner(#pathwayAnalysisId, PATHWAY_ANALYSIS) or isPermitted(anyOf('read:pathway','write:pathway')) or hasEntityAccess(#pathwayAnalysisId, PATHWAY_ANALYSIS, READ)) and (isPermitted('write:source') or hasSourceAccess(#sourceKey, WRITE))") + @PreAuthorize("(isOwner(#pathwayAnalysisId, PATHWAY_ANALYSIS) or isAnyPermitted(anyOf('read:pathway','write:pathway')) or hasEntityAccess(#pathwayAnalysisId, PATHWAY_ANALYSIS, READ)) and (isPermitted('write:source') or hasSourceAccess(#sourceKey, WRITE))") public void cancelPathwaysGeneration( @PathVariable("id") final Integer pathwayAnalysisId, @PathVariable("sourceKey") final String sourceKey @@ -330,7 +330,7 @@ public void cancelPathwaysGeneration( */ @GetMapping(value = "/{id}/generation", produces = MediaType.APPLICATION_JSON_VALUE) @Transactional - @PreAuthorize("isOwner(#pathwayAnalysisId, PATHWAY_ANALYSIS) or isPermitted(anyOf('read:pathway','write:pathway')) or hasEntityAccess(#pathwayAnalysisId, PATHWAY_ANALYSIS, READ)") + @PreAuthorize("isOwner(#pathwayAnalysisId, PATHWAY_ANALYSIS) or isAnyPermitted(anyOf('read:pathway','write:pathway')) or hasEntityAccess(#pathwayAnalysisId, PATHWAY_ANALYSIS, READ)") public List getPathwayGenerations( @PathVariable("id") final Integer pathwayAnalysisId ) { @@ -526,7 +526,7 @@ public void unassignPermissionProtectedTag(@PathVariable("id") final int id, @Pa * @return */ @GetMapping(value = "/{id}/version", produces = MediaType.APPLICATION_JSON_VALUE) - @PreAuthorize("isOwner(#id, PATHWAY_ANALYSIS) or isPermitted(anyOf('read:pathway','write:pathway')) or hasEntityAccess(#id, PATHWAY_ANALYSIS, READ)") + @PreAuthorize("isOwner(#id, PATHWAY_ANALYSIS) or isAnyPermitted(anyOf('read:pathway','write:pathway')) or hasEntityAccess(#id, PATHWAY_ANALYSIS, READ)") public List getVersions(@PathVariable("id") final long id) { return pathwayService.getVersions(id); } @@ -540,7 +540,7 @@ public List getVersions(@PathVariable("id") final long id) { * @return */ @GetMapping(value = "/{id}/version/{version}", produces = MediaType.APPLICATION_JSON_VALUE) - @PreAuthorize("isOwner(#id, PATHWAY_ANALYSIS) or isPermitted(anyOf('read:pathway','write:pathway')) or hasEntityAccess(#id, PATHWAY_ANALYSIS, READ)") + @PreAuthorize("isOwner(#id, PATHWAY_ANALYSIS) or isAnyPermitted(anyOf('read:pathway','write:pathway')) or hasEntityAccess(#id, PATHWAY_ANALYSIS, READ)") public PathwayVersionFullDTO getVersion(@PathVariable("id") final int id, @PathVariable("version") final int version) { return pathwayService.getVersion(id, version); } diff --git a/src/main/java/org/ohdsi/webapi/security/authc/LoginService.java b/src/main/java/org/ohdsi/webapi/security/authc/LoginService.java index e32f4f6226..b4c23e4d5b 100644 --- a/src/main/java/org/ohdsi/webapi/security/authc/LoginService.java +++ b/src/main/java/org/ohdsi/webapi/security/authc/LoginService.java @@ -111,7 +111,8 @@ public Result extend(Authentication authentication) { */ public Result logout(Authentication authentication) { if (!(authentication instanceof WebApiAuthenticationToken webapiAuth)) { - throw new BadCredentialsException("Invalid authentication type"); + String type = (authentication == null) ? "null" : authentication.getClass().getName(); + throw new BadCredentialsException("Invalid authentication type: " + type); } UUID sessionId = webapiAuth.getSessionId(); diff --git a/src/main/java/org/ohdsi/webapi/security/authz/AuthorizationCacheService.java b/src/main/java/org/ohdsi/webapi/security/authz/AuthorizationCacheService.java index 7ca90f25a3..3944f26f7d 100644 --- a/src/main/java/org/ohdsi/webapi/security/authz/AuthorizationCacheService.java +++ b/src/main/java/org/ohdsi/webapi/security/authz/AuthorizationCacheService.java @@ -82,6 +82,11 @@ public void evictUsersWithRole(Long roleId) { cache.removeAll(usersToEvict); } + public void evictUser(Long userId) { + Cache cache = authInfoCache(); + cache.remove(userId); + } + @CacheEvict(cacheNames = CachingSetup.AUTH_INFO_CACHE, allEntries = true) public void clearCache() { // no-op, clears cache diff --git a/src/main/java/org/ohdsi/webapi/security/authz/RoleService.java b/src/main/java/org/ohdsi/webapi/security/authz/RoleService.java index 474de8b29f..2d33a6bb1c 100644 --- a/src/main/java/org/ohdsi/webapi/security/authz/RoleService.java +++ b/src/main/java/org/ohdsi/webapi/security/authz/RoleService.java @@ -26,18 +26,20 @@ class RoleService { private final RoleRepository roleRepository; private final UserRoleRepository userRoleRepository; private final RolePermissionRepository rolePermissionRepository; - + private final AuthorizationCacheService authCacheService; public RoleService( RoleRepository roleRepository, UserRoleRepository userRoleRepository, RolePermissionRepository rolePermissionRepository, UserService userService, - PermissionService permissionService) { + PermissionService permissionService, + AuthorizationCacheService authCacheService) { this.roleRepository = roleRepository; this.userRoleRepository = userRoleRepository; this.rolePermissionRepository = rolePermissionRepository; this.userService = userService; this.permissionService = permissionService; + this.authCacheService = authCacheService; } // ------------------------- @@ -139,6 +141,7 @@ private RolePermissionEntity addPermission(final RoleEntity role, final Permissi relation.setRole(role); relation.setPermission(permission); relation = this.rolePermissionRepository.save(relation); + authCacheService.evictUsersWithRole(role.getId()); } return relation; @@ -149,6 +152,7 @@ public void removePermission(Long roleId, Long permissionId) { permissionId); if (rolePermission != null) this.rolePermissionRepository.delete(rolePermission); + authCacheService.evictUsersWithRole(roleId); } private Set getRolePermissions(RoleEntity role) { @@ -208,6 +212,7 @@ public UserRoleEntity addUserToRole(final UserEntity user, final RoleEntity role newRelation.setRole(role); newRelation.setOrigin(userOrigin != null ? userOrigin : UserOrigin.SYSTEM); newRelation = this.userRoleRepository.save(newRelation); + authCacheService.evictUser(user.getId()); return newRelation; }); @@ -228,6 +233,7 @@ public void removeUserFromRole(String login, String roleName, UserOrigin origin) .ifPresent((userRole) -> { if (origin == null || origin.equals(userRole.getOrigin())) { this.userRoleRepository.delete(userRole); + authCacheService.evictUser(user.getId()); } }); } @@ -237,7 +243,10 @@ public void removeUser(Long userId, Long roleId) { RoleEntity role = this.getRole(roleId); this.userRoleRepository.findByUserAndRole(user, role) - .ifPresent((userRole) -> this.userRoleRepository.delete(userRole)); + .ifPresent((userRole) -> { + this.userRoleRepository.delete(userRole); + authCacheService.evictUser(user.getId()); + }); } public Set getUserRoles(Long userId) { diff --git a/src/main/java/org/ohdsi/webapi/security/authz/UserController.java b/src/main/java/org/ohdsi/webapi/security/authz/UserController.java index 3ec74c0b42..f94ac534ce 100644 --- a/src/main/java/org/ohdsi/webapi/security/authz/UserController.java +++ b/src/main/java/org/ohdsi/webapi/security/authz/UserController.java @@ -4,7 +4,6 @@ import org.ohdsi.webapi.security.authz.access.UserAuthorizations; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; -import org.springframework.context.ApplicationEventPublisher; import org.springframework.http.MediaType; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.web.bind.annotation.*; @@ -28,9 +27,6 @@ private record UserInfo ( @Autowired private AuthorizationService authorizer; - @Autowired - private ApplicationEventPublisher eventPublisher; - @Value("${security.auth.ad.default.import.group}#{T(java.util.Collections).emptyList()}") private List defaultRoles; @@ -71,7 +67,6 @@ public ArrayList getUserRoles(@PathVariable("userId") Long userId) throws public Role createRole(@RequestBody Role role) throws Exception { RoleEntity roleEntity = this.authorizer.addRole(role.name(), true); Role newRole = Role.fromEntity(roleEntity); - eventPublisher.publishEvent(new AddRoleEvent(this, newRole.id(), newRole.name())); return newRole; } @@ -84,7 +79,6 @@ public Role updateRole(@PathVariable("roleId") Long id, @RequestBody Role role) } roleEntity.setName(role.name()); roleEntity = this.authorizer.updateRole(roleEntity); - eventPublisher.publishEvent(new ChangeRoleEvent(this, id, role.name())); return Role.fromEntity(roleEntity); } @@ -122,7 +116,6 @@ public void addPermissionToRole( @RequestParam List permissionIds) throws Exception { for (Long permissionId : permissionIds) { this.authorizer.addPermission(roleId, permissionId); - eventPublisher.publishEvent(new AddPermissionEvent(this, permissionId, roleId)); } } @@ -135,7 +128,6 @@ public void addPermissionToRole( for (String permissionIdString : ids) { Long permissionId = Long.parseLong(permissionIdString); this.authorizer.addPermission(roleId, permissionId); - eventPublisher.publishEvent(new AddPermissionEvent(this, permissionId, roleId)); } } @@ -148,7 +140,6 @@ public void removePermissionFromRole( for (String permissionIdString : ids) { Long permissionId = Long.parseLong(permissionIdString); this.authorizer.removePermission(roleId, permissionId); - eventPublisher.publishEvent(new DeletePermissionEvent(this, permissionId, roleId)); } } @@ -167,7 +158,6 @@ public void addUserToRole( for (String userIdString : ids) { Long userId = Long.parseLong(userIdString); this.authorizer.addUser(userId, roleId); - eventPublisher.publishEvent(new AssignRoleEvent(this, roleId, userId)); } } @@ -180,9 +170,7 @@ public void removeUserFromRole( for (String userIdString : ids) { Long userId = Long.parseLong(userIdString); this.authorizer.removeUser(userId, roleId); - eventPublisher.publishEvent(new UnassignRoleEvent(this, roleId, userId)); } } - } diff --git a/src/main/java/org/ohdsi/webapi/security/spring/WebApiSecurityExpressionRoot.java b/src/main/java/org/ohdsi/webapi/security/spring/WebApiSecurityExpressionRoot.java index 88b6920659..39b68e96e8 100644 --- a/src/main/java/org/ohdsi/webapi/security/spring/WebApiSecurityExpressionRoot.java +++ b/src/main/java/org/ohdsi/webapi/security/spring/WebApiSecurityExpressionRoot.java @@ -149,7 +149,7 @@ public boolean isPermitted(String permission) { * @param permission The permission string (e.g., "read:cohort", "write", "*") * @return true if the user has the permission */ - public boolean isPermitted(Collection permissions) { + public boolean isAnyPermitted(Collection permissions) { if (permissions == null || permissions.isEmpty()) return false; for (String p : permissions) { diff --git a/src/main/java/org/ohdsi/webapi/service/VocabularyService.java b/src/main/java/org/ohdsi/webapi/service/VocabularyService.java index 3ddffe94dd..1d33ade01b 100644 --- a/src/main/java/org/ohdsi/webapi/service/VocabularyService.java +++ b/src/main/java/org/ohdsi/webapi/service/VocabularyService.java @@ -29,8 +29,6 @@ import org.ohdsi.sql.SqlTranslate; import org.ohdsi.vocabulary.Concept; import org.ohdsi.vocabulary.SearchProviderConfig; -import org.ohdsi.webapi.activity.Activity.ActivityType; -import org.ohdsi.webapi.activity.Tracker; import org.ohdsi.webapi.conceptset.ConceptSetComparison; import org.ohdsi.webapi.conceptset.ConceptSetExport; import org.ohdsi.webapi.conceptset.ConceptSetOptimizationResult; @@ -1353,7 +1351,6 @@ public void clearCaches() { public Collection getDescendantOfAncestorConcepts( @PathVariable("sourceKey") String sourceKey, @RequestBody DescendentOfAncestorSearch search) { - Tracker.trackActivity(ActivityType.Search, "getDescendantOfAncestorConcepts"); Source source = getSourceRepository().findBySourceKey(sourceKey); PreparedStatementRenderer psr = prepareGetDescendantOfAncestorConcepts(search, source); @@ -1408,7 +1405,6 @@ public Collection getDescendantOfAncestorConcepts(@RequestBody Descende public Collection getRelatedConcepts( @PathVariable("sourceKey") String sourceKey, @RequestBody RelatedConceptSearch search) { - Tracker.trackActivity(ActivityType.Search, "getRelatedConcepts"); Source source = getSourceRepository().findBySourceKey(sourceKey); PreparedStatementRenderer psr = prepareGetRelatedConcepts(search, source); From 8d7784164e04dbc6f5cec5c476d82a70724187d6 Mon Sep 17 00:00:00 2001 From: Chris Knoll Date: Mon, 13 Apr 2026 17:39:17 -0400 Subject: [PATCH 2/2] Improved Etag impl to use Spring Boot ShallowEtagHeaderFilter. Adjusted CORS config to expose Etag header. --- articles/ETag_HOWTO.md | 36 +++---- .../webapi/security/authc/CorsConfig.java | 2 + .../org/ohdsi/webapi/util/EtagFilter.java | 101 +++++------------- .../java/org/ohdsi/webapi/util/EtagUtil.java | 82 -------------- src/main/resources/application.yaml | 4 + 5 files changed, 45 insertions(+), 180 deletions(-) delete mode 100644 src/main/java/org/ohdsi/webapi/util/EtagUtil.java diff --git a/articles/ETag_HOWTO.md b/articles/ETag_HOWTO.md index b640fac84e..b1fa2b75ef 100644 --- a/articles/ETag_HOWTO.md +++ b/articles/ETag_HOWTO.md @@ -24,12 +24,12 @@ That's it. No changes to return types or service layer required. 1. **First Request**: Client requests `/cohortdefinition/123` - Server generates response JSON - - Filter computes SHA-256 hash of response body - - Response includes `ETag: "a1b2c3..."` header + - Filter computes MD5 hash of response body + - Response includes weak ETag header: `ETag: W/"0a1b2c3..."` - Client receives full response (200 OK) 2. **Subsequent Requests**: Client requests same URL - - Browser automatically sends `If-None-Match: "a1b2c3..."` header + - Browser automatically sends `If-None-Match: W/"0a1b2c3..."` header - Filter computes ETag of current response - If ETags match → returns `304 Not Modified` (no body) - If ETags differ → returns full response with new ETag (200 OK) @@ -49,29 +49,18 @@ public @interface UseEtag { } ``` -### `EtagUtil` Utility Class - -Location: `org.ohdsi.webapi.util.EtagUtil` - -Provides ETag generation and comparison: - -- `generateEtag(byte[] content)` - Computes SHA-256 hash, returns quoted string per RFC 7232 (e.g., `"a1b2c3..."`) -- `matches(String ifNoneMatch, String etag)` - Compares `If-None-Match` header to generated ETag, handles multiple values and `*` wildcard - ### `EtagFilter` Servlet Filter Location: `org.ohdsi.webapi.util.EtagFilter` -A servlet `Filter` that: +Extends Spring's `ShallowEtagHeaderFilter` to add selective ETag processing based on the `@UseEtag` annotation: -1. Looks up the handler method via `RequestMappingHandlerMapping` -2. Checks for `@UseEtag` annotation -3. Wraps response with `ContentCachingResponseWrapper` to capture the body -4. After response is written, computes ETag from cached bytes -5. Compares with `If-None-Match` header -6. Returns 304 or full response with appropriate headers +1. Overrides `shouldNotFilter()` to skip requests without `@UseEtag` annotation +2. Sets Cache-Control and CORS headers before delegating to parent +3. Leverages Spring's built-in ETag generation (MD5) and 304 handling +4. Configured to generate weak ETags (`W/"..."`) per RFC 7232 -**Key Design Decision**: Uses a Filter (not `ResponseBodyAdvice`) to avoid double-serialization. `ResponseBodyAdvice` receives the Java object before JSON serialization, so computing an ETag there would require serializing to JSON twice. The Filter intercepts after Spring has already serialized the response. +**Key Design Decision**: Extends `ShallowEtagHeaderFilter` rather than implementing from scratch. This leverages Spring's tested implementation for response caching, ETag generation, async dispatch handling, and `If-None-Match` comparison. ## HTTP Headers @@ -79,7 +68,7 @@ For `@UseEtag` endpoints, the filter sets these response headers: | Header | Value | Purpose | |--------|-------|---------| -| `ETag` | `""` | Unique identifier for response content | +| `ETag` | `W/"0"` | Weak ETag - unique identifier for response content | | `Cache-Control` | `private, max-age=0, must-revalidate` | Allows browser caching but forces revalidation | | `Access-Control-Expose-Headers` | `ETag` | Exposes ETag to JavaScript in CORS contexts | @@ -184,7 +173,8 @@ Invoke-WebRequest: ParameterBindingException ## Security Considerations -- ETags use SHA-256 hashing, which is cryptographically strong +- ETags use MD5 hashing (sufficient for cache validation; collision resistance is not required) +- Weak ETags (`W/"..."`) indicate semantic equivalence rather than byte-for-byte identity - `Cache-Control: private` ensures responses are not stored in shared caches (proxies) - The `Vary: Origin` header (set by Spring CORS) ensures CORS responses are cached per-origin @@ -193,4 +183,4 @@ Invoke-WebRequest: ParameterBindingException - [RFC 7232 - HTTP Conditional Requests](https://tools.ietf.org/html/rfc7232) - [MDN - ETag](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ETag) - [MDN - If-None-Match](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-None-Match) -- [Spring ContentCachingResponseWrapper](https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/web/util/ContentCachingResponseWrapper.html) +- [Spring ShallowEtagHeaderFilter](https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/web/filter/ShallowEtagHeaderFilter.html) diff --git a/src/main/java/org/ohdsi/webapi/security/authc/CorsConfig.java b/src/main/java/org/ohdsi/webapi/security/authc/CorsConfig.java index 2ceed4ddfa..90a4d686e4 100644 --- a/src/main/java/org/ohdsi/webapi/security/authc/CorsConfig.java +++ b/src/main/java/org/ohdsi/webapi/security/authc/CorsConfig.java @@ -5,6 +5,7 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.http.HttpHeaders; import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.cors.CorsConfigurationSource; import org.springframework.web.cors.UrlBasedCorsConfigurationSource; @@ -23,6 +24,7 @@ public CorsConfigurationSource corsConfigurationSource(@Value("${security.cors.a config.setAllowedOrigins(Arrays.asList(allowedOrigins)); config.setAllowedMethods(Arrays.asList("GET", "POST", "PUT", "DELETE", "OPTIONS")); config.setAllowedHeaders(Arrays.asList("*")); + config.setExposedHeaders(Arrays.asList(HttpHeaders.ETAG)); config.setAllowCredentials(true); UrlBasedCorsConfigurationSource source = new UrlBasedCorsConfigurationSource(); diff --git a/src/main/java/org/ohdsi/webapi/util/EtagFilter.java b/src/main/java/org/ohdsi/webapi/util/EtagFilter.java index fcf84eca4e..5f6df74127 100644 --- a/src/main/java/org/ohdsi/webapi/util/EtagFilter.java +++ b/src/main/java/org/ohdsi/webapi/util/EtagFilter.java @@ -1,43 +1,40 @@ package org.ohdsi.webapi.util; -import jakarta.servlet.Filter; import jakarta.servlet.FilterChain; import jakarta.servlet.ServletException; -import jakarta.servlet.ServletRequest; -import jakarta.servlet.ServletResponse; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.core.Ordered; -import org.springframework.core.annotation.Order; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.http.HttpHeaders; -import org.springframework.http.HttpStatus; import org.springframework.stereotype.Component; +import org.springframework.web.filter.ShallowEtagHeaderFilter; import org.springframework.web.method.HandlerMethod; import org.springframework.web.servlet.HandlerExecutionChain; import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; -import org.springframework.web.util.ContentCachingResponseWrapper; import java.io.IOException; /** * Servlet filter that provides HTTP ETag support for endpoints annotated with {@link UseEtag}. *

+ * Extends Spring's {@link ShallowEtagHeaderFilter} to leverage its ETag generation and + * 304 Not Modified handling, while adding selective filtering based on the {@code @UseEtag} + * annotation. + *

+ *

* This filter: *

    - *
  • Looks up the handler method for the request
    • - *
  • If annotated with {@code @UseEtag}, wraps the response to capture the body
    • - *
  • After the response is written, computes an ETag from the body content
    • - *
  • If the client's {@code If-None-Match} header matches, returns 304 Not Modified
    • - *
  • Otherwise, adds the {@code ETag} header to the response
    • + *
  • Skips ETag processing for methods not annotated with {@code @UseEtag}
    • + *
  • Sets appropriate Cache-Control headers for browser caching with revalidation
    • + *
  • Exposes ETag header for CORS requests
    • + *
  • Generates weak ETags (W/"...") per RFC 7232
    • *
*

*/ @Component -@Order(Ordered.LOWEST_PRECEDENCE - 10) -public class EtagFilter implements Filter { +public class EtagFilter extends ShallowEtagHeaderFilter { private static final Logger LOG = LoggerFactory.getLogger(EtagFilter.class); @@ -45,29 +42,25 @@ public class EtagFilter implements Filter { public EtagFilter(@Qualifier("requestMappingHandlerMapping") RequestMappingHandlerMapping handlerMapping) { this.handlerMapping = handlerMapping; + setWriteWeakETag(true); } @Override - public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) - throws IOException, ServletException { - - HttpServletRequest httpRequest = (HttpServletRequest) request; - HttpServletResponse httpResponse = (HttpServletResponse) response; - - // Check if the handler method is annotated with @UseEtag - if (!hasUseEtagAnnotation(httpRequest)) { - chain.doFilter(request, response); - return; - } - - // Wrap response to capture the body - ContentCachingResponseWrapper wrappedResponse = new ContentCachingResponseWrapper(httpResponse); - - // Execute the filter chain (controller writes to wrapped response) - chain.doFilter(request, wrappedResponse); + protected boolean shouldNotFilter(HttpServletRequest request) { + return !hasUseEtagAnnotation(request); + } - // Process ETag after response is written - processEtag(httpRequest, wrappedResponse); + @Override + protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, + FilterChain filterChain) throws ServletException, IOException { + + // Override Spring Security's default "no-store" with ETag-compatible caching + // "private, max-age=0, must-revalidate" forces browser to cache but always revalidate + response.setHeader(HttpHeaders.CACHE_CONTROL, "private, max-age=0, must-revalidate"); + // Expose ETag header for CORS requests so browser/JS can access it + response.setHeader(HttpHeaders.ACCESS_CONTROL_EXPOSE_HEADERS, HttpHeaders.ETAG); + + super.doFilterInternal(request, response, filterChain); } private boolean hasUseEtagAnnotation(HttpServletRequest request) { @@ -86,46 +79,4 @@ private boolean hasUseEtagAnnotation(HttpServletRequest request) { } return false; } - - private void processEtag(HttpServletRequest request, ContentCachingResponseWrapper response) - throws IOException { - - byte[] content = response.getContentAsByteArray(); - - // Only process ETag for successful responses with content - int status = response.getStatus(); - if (!isSuccessStatus(status) || content.length == 0) { - response.copyBodyToResponse(); - return; - } - - String etag = EtagUtil.generateEtag(content); - if (etag == null) { - response.copyBodyToResponse(); - return; - } - - String ifNoneMatch = request.getHeader(HttpHeaders.IF_NONE_MATCH); - - // Override Spring Security's default "no-store" with ETag-compatible caching - // "private, max-age=0, must-revalidate" forces browser to cache but always revalidate - response.setHeader(HttpHeaders.CACHE_CONTROL, "private, max-age=0, must-revalidate"); - response.setHeader(HttpHeaders.ETAG, etag); - // Expose ETag header for CORS requests so browser/JS can access it - response.setHeader(HttpHeaders.ACCESS_CONTROL_EXPOSE_HEADERS, HttpHeaders.ETAG); - - if (EtagUtil.matches(ifNoneMatch, etag)) { - // Client has current version - return 304 without body - response.resetBuffer(); - response.setStatus(HttpStatus.NOT_MODIFIED.value()); - response.flushBuffer(); - } else { - // Return full response with ETag header - response.copyBodyToResponse(); - } - } - - private boolean isSuccessStatus(int status) { - return status >= 200 && status < 300; - } } diff --git a/src/main/java/org/ohdsi/webapi/util/EtagUtil.java b/src/main/java/org/ohdsi/webapi/util/EtagUtil.java deleted file mode 100644 index 48af994840..0000000000 --- a/src/main/java/org/ohdsi/webapi/util/EtagUtil.java +++ /dev/null @@ -1,82 +0,0 @@ -package org.ohdsi.webapi.util; - -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; - -/** - * Utility class for generating and comparing HTTP ETags. - */ -public final class EtagUtil { - - private static final char[] HEX_CHARS = "0123456789abcdef".toCharArray(); - - private EtagUtil() { - // Utility class - } - - /** - * Generates a quoted ETag from the given content bytes using SHA-256. - * - * @param content the response body bytes - * @return a quoted ETag string (e.g., {@code "a1b2c3d4..."}) - */ - public static String generateEtag(byte[] content) { - if (content == null || content.length == 0) { - return null; - } - try { - MessageDigest digest = MessageDigest.getInstance("SHA-256"); - byte[] hash = digest.digest(content); - return "\"" + bytesToHex(hash) + "\""; - } catch (NoSuchAlgorithmException e) { - throw new IllegalStateException("SHA-256 algorithm not available", e); - } - } - - /** - * Checks if the {@code If-None-Match} header value matches the given ETag. - *

- * Supports multiple comma-separated ETags and the wildcard {@code *}. - *

- * - * @param ifNoneMatch the value of the If-None-Match header (may be null) - * @param etag the generated ETag to compare against - * @return true if the ETag matches and a 304 response should be returned - */ - public static boolean matches(String ifNoneMatch, String etag) { - if (ifNoneMatch == null || ifNoneMatch.isBlank() || etag == null) { - return false; - } - - String trimmed = ifNoneMatch.trim(); - - // Wildcard matches any ETag - if ("*".equals(trimmed)) { - return true; - } - - // Handle multiple ETags: "etag1", "etag2", "etag3" - for (String candidate : trimmed.split(",")) { - String normalized = candidate.trim(); - // Handle weak ETags (W/"...") by stripping the prefix for comparison - if (normalized.startsWith("W/")) { - normalized = normalized.substring(2); - } - if (normalized.equals(etag)) { - return true; - } - } - - return false; - } - - private static String bytesToHex(byte[] bytes) { - char[] hexChars = new char[bytes.length * 2]; - for (int i = 0; i < bytes.length; i++) { - int v = bytes[i] & 0xFF; - hexChars[i * 2] = HEX_CHARS[v >>> 4]; - hexChars[i * 2 + 1] = HEX_CHARS[v & 0x0F]; - } - return new String(hexChars); - } -} diff --git a/src/main/resources/application.yaml b/src/main/resources/application.yaml index b410f6927e..cd26fbc6a8 100644 --- a/src/main/resources/application.yaml +++ b/src/main/resources/application.yaml @@ -294,6 +294,10 @@ sensitiveinfo: # EMBEDDED SERVER CONFIGURATION (ServerProperties) server: + compression: + enabled: true + mime-types: application/json,application/xml,text/html,text/xml,text/plain + min-response-size: 1024 servlet: context-path: /WebAPI port: 8080