Skip to content

Commit 0f4d63d

Browse files
chaitalicodchaitalithombare
authored andcommitted
ATLAS-5372: Prevent cross-user overwrite in saved search create API via caller-supplied guid (#724)
(Cherrypicked from c1cc1ff) Co-authored-by: chaitalithombare <chaitalithombare@apache.org>
1 parent 95efd62 commit 0f4d63d

2 files changed

Lines changed: 85 additions & 0 deletions

File tree

repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -699,6 +699,12 @@ public AtlasUserSavedSearch addSavedSearch(String currentUser, AtlasUserSavedSea
699699

700700
checkSavedSearchOwnership(currentUser, savedSearch);
701701

702+
if (StringUtils.isNotBlank(savedSearch.getGuid())) {
703+
AtlasUserSavedSearch existingSavedSearch = userProfileService.getSavedSearch(savedSearch.getGuid());
704+
705+
checkSavedSearchOwnership(currentUser, existingSavedSearch);
706+
}
707+
702708
return userProfileService.addSavedSearch(savedSearch);
703709
} catch (AtlasBaseException e) {
704710
LOG.error("addSavedSearch({})", savedSearch, e);

repository/src/test/java/org/apache/atlas/discovery/EntityDiscoveryServiceTest.java

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.apache.atlas.discovery;
1919

2020
import org.apache.atlas.AtlasConfiguration;
21+
import org.apache.atlas.AtlasErrorCode;
2122
import org.apache.atlas.RequestContext;
2223
import org.apache.atlas.exception.AtlasBaseException;
2324
import org.apache.atlas.model.discovery.AtlasQuickSearchResult;
@@ -60,9 +61,13 @@
6061
import static org.mockito.ArgumentMatchers.anyString;
6162
import static org.mockito.Mockito.mock;
6263
import static org.mockito.Mockito.mockStatic;
64+
import static org.mockito.Mockito.never;
65+
import static org.mockito.Mockito.verify;
6366
import static org.mockito.Mockito.when;
67+
import static org.testng.Assert.assertEquals;
6468
import static org.testng.Assert.assertNotNull;
6569
import static org.testng.Assert.assertTrue;
70+
import static org.testng.Assert.fail;
6671

6772
public class EntityDiscoveryServiceTest {
6873
@Mock
@@ -149,6 +154,32 @@ private void setupMockBehaviors() throws AtlasBaseException {
149154
when(entityDiscoveryService.searchGUIDsWithParameters(any(AtlasAuditAgingType.class), any(Set.class), any(SearchParameters.class))).thenReturn(new HashSet<>());
150155
}
151156

157+
private EntityDiscoveryService createStrictServiceForSecurityTests() throws Exception {
158+
/*
159+
* Strict setup path for security-sensitive tests:
160+
* always instantiate a real EntityDiscoveryService and never
161+
* fallback to a mocked/spied service.
162+
*/
163+
when(typeRegistry.getEntityTypeByName(anyString())).thenReturn(entityType);
164+
when(entityType.getTypeAndAllSubTypesQryStr()).thenReturn("(TestType)");
165+
when(entityType.getAttribute(anyString())).thenReturn(attribute);
166+
when(entityType.getTypeName()).thenReturn("TestType");
167+
when(attribute.getVertexPropertyName()).thenReturn("v.testAttr");
168+
when(attribute.getAttributeType()).thenReturn(mock(org.apache.atlas.type.AtlasType.class));
169+
when(indexer.getVertexIndexKeys()).thenReturn(Collections.emptySet());
170+
when(indexer.getEdgeIndexKeys()).thenReturn(Collections.emptySet());
171+
when(graph.indexQuery(anyString(), anyString())).thenReturn(indexQuery);
172+
when(graph.query()).thenReturn(mock(org.apache.atlas.repository.graphdb.AtlasGraphQuery.class));
173+
when(indexQuery.vertices()).thenReturn(Collections.emptyIterator());
174+
when(indexQuery.vertexTotals()).thenReturn(0L);
175+
176+
RequestContext context = mock(RequestContext.class);
177+
when(RequestContext.get()).thenReturn(context);
178+
when(context.getUser()).thenReturn("testUser");
179+
180+
return new EntityDiscoveryService(typeRegistry, graph, indexer, searchTracker, userProfileService, taskManagement);
181+
}
182+
152183
@AfterMethod
153184
public void tearDown() {
154185
if (atlasConfigurationMock != null) {
@@ -255,6 +286,54 @@ public void testUpdateSavedSearch() throws AtlasBaseException {
255286
}
256287
}
257288

289+
@Test
290+
public void testAddSavedSearchWithGuidFromAnotherUserIsRejectedStrict() throws Exception {
291+
EntityDiscoveryService strictService = createStrictServiceForSecurityTests();
292+
AtlasUserSavedSearch savedSearch = new AtlasUserSavedSearch();
293+
294+
savedSearch.setGuid("testGuid");
295+
savedSearch.setOwnerName("testUser");
296+
savedSearch.setName("testSearch");
297+
savedSearch.setSearchType(AtlasUserSavedSearch.SavedSearchType.BASIC);
298+
299+
AtlasUserSavedSearch existingSavedSearch = new AtlasUserSavedSearch();
300+
existingSavedSearch.setGuid("testGuid");
301+
existingSavedSearch.setOwnerName("otherUser");
302+
303+
when(userProfileService.getSavedSearch("testGuid")).thenReturn(existingSavedSearch);
304+
305+
try {
306+
strictService.addSavedSearch("testUser", savedSearch);
307+
fail("Expected cross-user GUID reuse to be rejected");
308+
} catch (AtlasBaseException e) {
309+
assertEquals(e.getAtlasErrorCode(), AtlasErrorCode.BAD_REQUEST);
310+
verify(userProfileService, never()).addSavedSearch(any(AtlasUserSavedSearch.class));
311+
}
312+
}
313+
314+
@Test
315+
public void testAddSavedSearchWithOwnGuidIsAllowedStrict() throws Exception {
316+
EntityDiscoveryService strictService = createStrictServiceForSecurityTests();
317+
AtlasUserSavedSearch savedSearch = new AtlasUserSavedSearch();
318+
319+
savedSearch.setGuid("testGuid");
320+
savedSearch.setOwnerName("testUser");
321+
savedSearch.setName("testSearch");
322+
savedSearch.setSearchType(AtlasUserSavedSearch.SavedSearchType.BASIC);
323+
324+
AtlasUserSavedSearch existingSavedSearch = new AtlasUserSavedSearch();
325+
existingSavedSearch.setGuid("testGuid");
326+
existingSavedSearch.setOwnerName("testUser");
327+
328+
when(userProfileService.getSavedSearch("testGuid")).thenReturn(existingSavedSearch);
329+
when(userProfileService.addSavedSearch(savedSearch)).thenReturn(savedSearch);
330+
331+
AtlasUserSavedSearch result = strictService.addSavedSearch("testUser", savedSearch);
332+
333+
assertEquals(result, savedSearch);
334+
verify(userProfileService).addSavedSearch(savedSearch);
335+
}
336+
258337
@Test
259338
public void testDeleteSavedSearch() throws AtlasBaseException {
260339
try {

0 commit comments

Comments
 (0)