Skip to content

Commit 2bd7e21

Browse files
fix: merge sensitive data when updating pages with sanitized fetcher config
(cherry picked from commit d86b31d)
1 parent a667f2b commit 2bd7e21

File tree

6 files changed

+222
-7
lines changed

6 files changed

+222
-7
lines changed

gravitee-apim-rest-api/gravitee-apim-rest-api-service/src/main/java/io/gravitee/apim/core/documentation/domain_service/PageSourceDomainService.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,4 +26,6 @@ public interface PageSourceDomainService {
2626
void setContentFromSource(Page page);
2727

2828
void removeSensitiveData(Page page);
29+
30+
void mergeSensitiveData(Page oldPage, Page newPage);
2931
}

gravitee-apim-rest-api/gravitee-apim-rest-api-service/src/main/java/io/gravitee/apim/core/documentation/use_case/ApiUpdateDocumentationPageUseCase.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import io.gravitee.apim.core.documentation.domain_service.ApiDocumentationDomainService;
2323
import io.gravitee.apim.core.documentation.domain_service.DocumentationValidationDomainService;
2424
import io.gravitee.apim.core.documentation.domain_service.HomepageDomainService;
25+
import io.gravitee.apim.core.documentation.domain_service.PageSourceDomainService;
2526
import io.gravitee.apim.core.documentation.domain_service.UpdateApiDocumentationDomainService;
2627
import io.gravitee.apim.core.documentation.model.AccessControl;
2728
import io.gravitee.apim.core.documentation.model.Page;
@@ -45,6 +46,7 @@ public class ApiUpdateDocumentationPageUseCase {
4546
private final PageCrudService pageCrudService;
4647
private final PageQueryService pageQueryService;
4748
private final DocumentationValidationDomainService documentationValidationDomainService;
49+
private final PageSourceDomainService pageSourceDomainService;
4850

4951
public Output execute(Input input) {
5052
var api = this.apiCrudService.get(input.apiId);
@@ -77,6 +79,10 @@ public Output execute(Input input) {
7779

7880
var pageToUpdate = newPage.build();
7981

82+
if (pageToUpdate.getSource() != null && oldPage.getSource() != null) {
83+
this.pageSourceDomainService.mergeSensitiveData(oldPage, pageToUpdate);
84+
}
85+
8086
if (!Objects.equals(oldPage.getContent(), input.content) || !Objects.equals(oldPage.getSource(), input.source)) {
8187
pageToUpdate = this.documentationValidationDomainService.validateAndSanitizeForUpdate(
8288
pageToUpdate,

gravitee-apim-rest-api/gravitee-apim-rest-api-service/src/main/java/io/gravitee/apim/infra/domain_service/documentation/PageSourceDomainServiceImpl.java

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,49 @@ public void removeSensitiveData(Page page) {
8080
});
8181
}
8282

83+
@Override
84+
public void mergeSensitiveData(Page oldPage, Page newPage) {
85+
if (oldPage.getSource() == null || newPage.getSource() == null) {
86+
return;
87+
}
88+
if (oldPage.getSource().getConfiguration() == null || newPage.getSource().getConfiguration() == null) {
89+
return;
90+
}
91+
92+
var oldFetcherOpt = loadFetcher(oldPage);
93+
var newFetcherOpt = loadFetcher(newPage);
94+
95+
if (oldFetcherOpt.isPresent() && newFetcherOpt.isPresent()) {
96+
var oldFetcher = oldFetcherOpt.get();
97+
var newFetcher = newFetcherOpt.get();
98+
99+
FetcherConfiguration originalFetcherConfiguration = oldFetcher.getConfiguration();
100+
FetcherConfiguration updatedFetcherConfiguration = newFetcher.getConfiguration();
101+
boolean updated = false;
102+
103+
Field[] fields = originalFetcherConfiguration.getClass().getDeclaredFields();
104+
for (Field field : fields) {
105+
if (field.isAnnotationPresent(Sensitive.class)) {
106+
boolean accessible = field.isAccessible();
107+
field.setAccessible(true);
108+
try {
109+
Object updatedValue = field.get(updatedFetcherConfiguration);
110+
if (SENSITIVE_DATA_REPLACEMENT.equals(updatedValue)) {
111+
updated = true;
112+
field.set(updatedFetcherConfiguration, field.get(originalFetcherConfiguration));
113+
}
114+
} catch (IllegalAccessException | IllegalArgumentException e) {
115+
log.error("Error while merging original fetcher sensitive data to new fetcher", e);
116+
}
117+
field.setAccessible(accessible);
118+
}
119+
}
120+
if (updated) {
121+
newPage.getSource().setConfiguration((new ObjectMapper().valueToTree(updatedFetcherConfiguration).toString()));
122+
}
123+
}
124+
}
125+
83126
private void fetchContent(Fetcher fetcher, Page page) {
84127
if (page.getType() != Page.Type.ROOT) {
85128
page.setContent(readContent(fetcher, page.getSource()));

gravitee-apim-rest-api/gravitee-apim-rest-api-service/src/test/java/inmemory/PageSourceDomainServiceInMemory.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,22 @@ public void removeSensitiveData(Page page) {
5050
);
5151
}
5252
}
53+
54+
@Override
55+
public void mergeSensitiveData(Page oldPage, Page newPage) {
56+
if (oldPage.getSource() == null || newPage.getSource() == null) {
57+
return;
58+
}
59+
if (oldPage.getSource().getConfiguration() == null || newPage.getSource().getConfiguration() == null) {
60+
return;
61+
}
62+
63+
String newConfig = newPage.getSource().getConfiguration();
64+
String oldConfig = oldPage.getSource().getConfiguration();
65+
String sanitizedValue = "\"" + PageSourceDomainServiceImpl.SENSITIVE_DATA_REPLACEMENT + "\"";
66+
67+
if (newConfig.contains(sanitizedValue) && oldConfig.contains("I'm a sensitive data")) {
68+
newPage.getSource().setConfiguration(newConfig.replace(sanitizedValue, "\"I'm a sensitive data\""));
69+
}
70+
}
5371
}

gravitee-apim-rest-api/gravitee-apim-rest-api-service/src/test/java/io/gravitee/apim/core/documentation/use_case/ApiUpdateDocumentationPageUseCaseTest.java

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,8 @@ void setUp() {
206206
apiCrudService,
207207
pageCrudService,
208208
pageQueryService,
209-
documentationValidationDomainService
209+
documentationValidationDomainService,
210+
pageSourceDomainService
210211
);
211212
apiCrudService.initWith(List.of(ApiFixtures.aProxyApiV4().toBuilder().id(API_ID).build()));
212213
roleQueryService.initWith(
@@ -513,6 +514,50 @@ void should_update_page_source() {
513514
assertThat(res.page()).isNotNull().hasFieldOrPropertyWithValue("id", PAGE_ID).hasFieldOrPropertyWithValue("source", pageSource);
514515
}
515516

517+
@Test
518+
void should_merge_sensitive_data_from_old_page_when_updating_with_masked_source() {
519+
initApiServices(List.of(Api.builder().id(API_ID).build()));
520+
521+
var oldPageWithSensitiveData = OLD_MARKDOWN_PAGE.toBuilder()
522+
.source(
523+
PageSource.builder()
524+
.type("http-fetcher")
525+
.configuration("{\"url\": \"https://example.com\", \"token\": \"I'm a sensitive data\"}")
526+
.build()
527+
)
528+
.build();
529+
530+
initPageServices(List.of(PARENT_FOLDER, oldPageWithSensitiveData));
531+
532+
var updatedPageSourceWithMaskedData = PageSource.builder()
533+
.type("http-fetcher")
534+
.configuration("{\"url\": \"https://example.com\", \"token\": \"********\"}")
535+
.build();
536+
537+
var res = apiUpdateDocumentationPageUsecase.execute(
538+
ApiUpdateDocumentationPageUseCase.Input.builder()
539+
.source(updatedPageSourceWithMaskedData)
540+
.apiId(API_ID)
541+
.pageId(OLD_MARKDOWN_PAGE.getId())
542+
.order(OLD_MARKDOWN_PAGE.getOrder())
543+
.visibility(OLD_MARKDOWN_PAGE.getVisibility())
544+
.content(OLD_MARKDOWN_PAGE.getContent())
545+
.name(OLD_MARKDOWN_PAGE.getName())
546+
.auditInfo(AUDIT_INFO)
547+
.build()
548+
);
549+
550+
assertThat(res.page())
551+
.isNotNull()
552+
.hasFieldOrPropertyWithValue("id", PAGE_ID)
553+
.extracting(Page::getSource)
554+
.isNotNull()
555+
.extracting(PageSource::getConfiguration)
556+
.asString()
557+
.contains("I'm a sensitive data")
558+
.doesNotContain("********");
559+
}
560+
516561
@Test
517562
void should_throw_error_if_markdown_unsafe() {
518563
initApiServices(List.of(Api.builder().id(API_ID).build()));

gravitee-apim-rest-api/gravitee-apim-rest-api-service/src/test/java/io/gravitee/apim/infra/domain_service/documentation/PageSourceDomainServiceImplTest.java

Lines changed: 107 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,16 +100,117 @@ void should_remove_sensitive_data() throws JsonProcessingException {
100100
assertThat(configuration.get("nonSensitiveData").textValue()).isEqualTo("I'm not a sensitive data");
101101
}
102102

103+
@Test
104+
@SuppressWarnings("unchecked")
105+
void should_merge_sensitive_data_from_old_page_to_new_page() throws JsonProcessingException {
106+
// Arrange
107+
String originalSensitiveData = "original-secret-token";
108+
String maskedSensitiveData = PageSourceDomainServiceImpl.SENSITIVE_DATA_REPLACEMENT;
109+
String nonSensitiveData = "I'm not a sensitive data";
110+
111+
PageSource oldPageSource = dummySource(nonSensitiveData, originalSensitiveData);
112+
PageSource newPageSource = dummySource(nonSensitiveData, maskedSensitiveData);
113+
114+
Page oldPage = Page.builder().source(oldPageSource).build();
115+
Page newPage = Page.builder().source(newPageSource).build();
116+
117+
when(applicationContext.getAutowireCapableBeanFactory()).thenReturn(
118+
mock(org.springframework.beans.factory.config.AutowireCapableBeanFactory.class)
119+
);
120+
when(fetcherPlugin.fetcher()).thenReturn(DummyFetcher.class);
121+
when(fetcherPlugin.configuration()).thenReturn(DummyFetcherConfiguration.class);
122+
when(fetcherPlugin.clazz()).thenReturn("io.gravitee.apim.infra.domain_service.documentation.DummyFetcher");
123+
when(pluginManager.get("dummy-fetcher")).thenReturn(fetcherPlugin);
124+
125+
// Mock fetcher configuration factory to return appropriate configurations
126+
// First call for old page, second call for new page
127+
when(fetcherConfigurationFactory.create(any(), any()))
128+
.thenReturn(new DummyFetcherConfiguration(nonSensitiveData, originalSensitiveData))
129+
.thenReturn(new DummyFetcherConfiguration(nonSensitiveData, maskedSensitiveData));
130+
131+
// Act
132+
cut.mergeSensitiveData(oldPage, newPage);
133+
134+
// Assert
135+
JsonNode configuration = new ObjectMapper().readTree(newPage.getSource().getConfiguration());
136+
assertThat(configuration.get("sensitiveData").textValue()).isEqualTo(originalSensitiveData);
137+
assertThat(configuration.get("nonSensitiveData").textValue()).isEqualTo(nonSensitiveData);
138+
}
139+
140+
@Test
141+
void should_not_merge_when_old_page_has_no_source() {
142+
Page oldPage = Page.builder().build();
143+
Page newPage = Page.builder().source(dummySource()).build();
144+
145+
cut.mergeSensitiveData(oldPage, newPage);
146+
147+
verifyNoInteractions(fetcherConfigurationFactory, pluginManager, applicationContext);
148+
}
149+
150+
@Test
151+
void should_not_merge_when_new_page_has_no_source() {
152+
Page oldPage = Page.builder().source(dummySource()).build();
153+
Page newPage = Page.builder().build();
154+
155+
cut.mergeSensitiveData(oldPage, newPage);
156+
157+
verifyNoInteractions(fetcherConfigurationFactory, pluginManager, applicationContext);
158+
}
159+
160+
@Test
161+
void should_not_merge_when_sensitive_data_is_not_masked() throws JsonProcessingException {
162+
// Arrange
163+
String originalSensitiveData = "original-secret-token";
164+
String newSensitiveData = "new-secret-token";
165+
String nonSensitiveData = "I'm not a sensitive data";
166+
167+
PageSource oldPageSource = dummySource(nonSensitiveData, originalSensitiveData);
168+
PageSource newPageSource = dummySource(nonSensitiveData, newSensitiveData);
169+
170+
Page oldPage = Page.builder().source(oldPageSource).build();
171+
Page newPage = Page.builder().source(newPageSource).build();
172+
173+
when(applicationContext.getAutowireCapableBeanFactory()).thenReturn(
174+
mock(org.springframework.beans.factory.config.AutowireCapableBeanFactory.class)
175+
);
176+
when(fetcherPlugin.fetcher()).thenReturn(DummyFetcher.class);
177+
when(fetcherPlugin.configuration()).thenReturn(DummyFetcherConfiguration.class);
178+
when(fetcherPlugin.clazz()).thenReturn("io.gravitee.apim.infra.domain_service.documentation.DummyFetcher");
179+
when(pluginManager.get("dummy-fetcher")).thenReturn(fetcherPlugin);
180+
181+
// Mock fetcher configuration factory to return appropriate configurations
182+
// First call for old page, second call for new page
183+
when(fetcherConfigurationFactory.create(any(), any()))
184+
.thenReturn(new DummyFetcherConfiguration(nonSensitiveData, originalSensitiveData))
185+
.thenReturn(new DummyFetcherConfiguration(nonSensitiveData, newSensitiveData));
186+
187+
// Act
188+
cut.mergeSensitiveData(oldPage, newPage);
189+
190+
// Assert - new sensitive data should remain unchanged since it's not masked
191+
JsonNode configuration = new ObjectMapper().readTree(newPage.getSource().getConfiguration());
192+
assertThat(configuration.get("sensitiveData").textValue()).isEqualTo(newSensitiveData);
193+
assertThat(configuration.get("nonSensitiveData").textValue()).isEqualTo(nonSensitiveData);
194+
}
195+
103196
private static PageSource dummySource() {
197+
return dummySource("I'm not a sensitive data", "I'm a sensitive data, I should be masked");
198+
}
199+
200+
private static PageSource dummySource(String nonSensitiveData, String sensitiveData) {
104201
return PageSource.builder()
105202
.type("dummy-fetcher")
106203
.configuration(
107-
"""
108-
{
109-
"sensitiveData" : I'm a sensitive data, I should be masked,
110-
"nonSensitiveData" : "I'm not a sensitive data"
111-
}
112-
"""
204+
String.format(
205+
"""
206+
{
207+
"nonSensitiveData" : "%s",
208+
"sensitiveData" : "%s"
209+
}
210+
""",
211+
nonSensitiveData,
212+
sensitiveData
213+
)
113214
)
114215
.build();
115216
}

0 commit comments

Comments
 (0)