Skip to content

Commit 1dbe06e

Browse files
Completes FlowFrameworkSecureRestApiIT
Signed-off-by: Darshit Chanpura <[email protected]>
1 parent b57941a commit 1dbe06e

File tree

1 file changed

+64
-18
lines changed

1 file changed

+64
-18
lines changed

src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java

Lines changed: 64 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,12 @@ public void testGetWorkflowStepsWithReadAccess() throws Exception {
179179
public void testGetWorkflowWithReadAccess() throws Exception {
180180
// No permissions to create, so we assert only that the response status isnt forbidden
181181
ResponseException exception = expectThrows(ResponseException.class, () -> getWorkflow(bobClient, "test"));
182-
assertEquals(RestStatus.NOT_FOUND, TestHelpers.restStatus(exception.getResponse()));
182+
if (isResourceSharingFeatureEnabled()) {
183+
// 403 because resource is not shared
184+
assertEquals(RestStatus.FORBIDDEN.getStatus(), exception.getResponse().getStatusLine().getStatusCode());
185+
} else {
186+
assertEquals(RestStatus.NOT_FOUND, TestHelpers.restStatus(exception.getResponse()));
187+
}
183188
}
184189

185190
public void testFilterByDisabled() throws Exception {
@@ -188,8 +193,15 @@ public void testFilterByDisabled() throws Exception {
188193
Response aliceWorkflow = createWorkflow(aliceClient, template);
189194
Map<String, Object> responseMap = entityAsMap(aliceWorkflow);
190195
String workflowId = (String) responseMap.get(WORKFLOW_ID);
191-
Response response = getWorkflow(catClient, workflowId);
192-
assertEquals(RestStatus.OK, TestHelpers.restStatus(response));
196+
197+
// when resources sharing is enabled, we throw 403
198+
if (isResourceSharingFeatureEnabled()) {
199+
ResponseException exception = expectThrows(ResponseException.class, () -> getWorkflow(catClient, workflowId));
200+
assertEquals(exception.getResponse().getStatusLine().getStatusCode(), RestStatus.FORBIDDEN.getStatus());
201+
} else {
202+
Response response = getWorkflow(catClient, workflowId);
203+
assertEquals(RestStatus.OK, TestHelpers.restStatus(response));
204+
}
193205
}
194206

195207
public void testSearchWorkflowWithReadAccess() throws Exception {
@@ -212,9 +224,15 @@ public void testGetWorkflowStateWithReadAccess() throws Exception {
212224

213225
Map<String, Object> responseMap = entityAsMap(response);
214226
String workflowId = (String) responseMap.get(WORKFLOW_ID);
215-
// No permissions to create or provision, so we assert only that the response status isnt forbidden
216-
Response searchResponse = getWorkflowStatus(bobClient, workflowId, false);
217-
assertEquals(RestStatus.OK, TestHelpers.restStatus(searchResponse));
227+
if (isResourceSharingFeatureEnabled()) {
228+
ResponseException exception = expectThrows(ResponseException.class, () -> getWorkflowStatus(bobClient, workflowId, false));
229+
assertTrue(exception.getMessage().contains("Failed to get workflow status"));
230+
assertEquals(exception.getResponse().getStatusLine().getStatusCode(), RestStatus.FORBIDDEN.getStatus());
231+
} else {
232+
// No permissions to create or provision, so we assert only that the response status isnt forbidden
233+
Response searchResponse = getWorkflowStatus(bobClient, workflowId, false);
234+
assertEquals(RestStatus.OK, TestHelpers.restStatus(searchResponse));
235+
}
218236
}
219237

220238
public void testSearchWorkflowStateWithReadAccess() throws Exception {
@@ -226,18 +244,26 @@ public void testSearchWorkflowStateWithReadAccess() throws Exception {
226244
// No permissions to create, so we assert only that the response status isnt forbidden
227245
String termIdQuery = "{\"query\":{\"bool\":{\"filter\":[{\"term\":{\"ids\":\"test\"}}]}}}";
228246
SearchResponse searchResponse = searchWorkflowState(bobClient, termIdQuery);
247+
// when resource sharing is enabled this would return an empty response
229248
assertEquals(RestStatus.OK, searchResponse.status());
230249
}
231250

232-
public void testCreateWorkflowWithNoBackendRole() throws IOException {
251+
public void testCreateWorkflowWithNoBackendRole() throws Exception {
233252
enableFilterBy();
234253
// User Dog has FF full access, but has no backend role
235254
// When filter by is enabled, we block creating workflows
236255
Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json");
237-
Exception exception = expectThrows(IOException.class, () -> { createWorkflow(dogClient, template); });
238-
assertTrue(
239-
exception.getMessage().contains("Filter by backend roles is enabled, but User dog does not have any backend roles configured")
240-
);
256+
if (isResourceSharingFeatureEnabled()) {
257+
// If resource sharing is enabled, we allow creation of workflows regardless of the user's backend roles
258+
Response dogWorkflow = createWorkflow(dogClient, template);
259+
assertEquals(RestStatus.CREATED, TestHelpers.restStatus(dogWorkflow));
260+
} else {
261+
Exception exception = expectThrows(IOException.class, () -> { createWorkflow(dogClient, template); });
262+
assertTrue(
263+
exception.getMessage()
264+
.contains("Filter by backend roles is enabled, but User dog does not have any backend roles configured")
265+
);
266+
}
241267
}
242268

243269
public void testDeprovisionWorkflowWithWriteAccess() throws Exception {
@@ -259,7 +285,12 @@ public void testGetWorkflowWithFilterEnabled() throws Exception {
259285
String workflowId = (String) responseMap.get(WORKFLOW_ID);
260286
// User Cat has FF full access, but is part of different backend role so Cat should not be able to access alice workflow
261287
ResponseException exception = expectThrows(ResponseException.class, () -> getWorkflow(catClient, workflowId));
262-
assertTrue(exception.getMessage().contains("User does not have permissions to access workflow: " + workflowId));
288+
if (isResourceSharingFeatureEnabled()) {
289+
assertTrue(exception.getMessage().contains("Failed to get workflow."));
290+
assertEquals(exception.getResponse().getStatusLine().getStatusCode(), RestStatus.FORBIDDEN.getStatus());
291+
} else {
292+
assertTrue(exception.getMessage().contains("User does not have permissions to access workflow: " + workflowId));
293+
}
263294
}
264295

265296
public void testGetWorkflowFilterbyEnabledForAdmin() throws Exception {
@@ -402,7 +433,15 @@ public void testCreateProvisionDeprovisionWorkflowWithFullAccess() throws Except
402433

403434
// Invoke status API with failure
404435
ResponseException exception = expectThrows(ResponseException.class, () -> getWorkflowStatus(aliceClient, workflowId, false));
405-
assertEquals(RestStatus.NOT_FOUND.getStatus(), exception.getResponse().getStatusLine().getStatusCode());
436+
if (isResourceSharingFeatureEnabled()) {
437+
// sample log message: No sharing info found for 'pmaAdZoBL3PmSyJoha0C'. Action
438+
// cluster:admin/opensearch/flow_framework/workflow_state/get is not allowed.
439+
// since record is deleted corresponding sharing record will also be deleted and thus we show 403 for non-existent sharing
440+
// records,
441+
assertEquals(RestStatus.FORBIDDEN.getStatus(), exception.getResponse().getStatusLine().getStatusCode());
442+
} else {
443+
assertEquals(RestStatus.NOT_FOUND.getStatus(), exception.getResponse().getStatusLine().getStatusCode());
444+
}
406445
}
407446

408447
public void testUpdateWorkflowEnabledForAdmin() throws Exception {
@@ -470,11 +509,18 @@ public void testUpdateWorkflowWithFilterEnabled() throws Exception {
470509
String workflowId = (String) responseMap.get(WORKFLOW_ID);
471510

472511
enableFilterBy();
473-
// User Fish has FF full access, and has "odfe" backend role which is one of Alice's backend role, so
474-
// Fish should be able to update workflows created by Alice. But the workflow's backend role should
475-
// not be replaced as Fish's backend roles.
476-
Response updateResponse = updateWorkflow(fishClient, workflowId, template);
477-
assertEquals(RestStatus.CREATED, TestHelpers.restStatus(updateResponse));
512+
if (isResourceSharingFeatureEnabled()) {
513+
ResponseException exception = expectThrows(ResponseException.class, () -> updateWorkflow(fishClient, workflowId, template));
514+
// we show 403 because fishClient doesn't have resource access
515+
assertEquals(RestStatus.FORBIDDEN.getStatus(), exception.getResponse().getStatusLine().getStatusCode());
516+
// it should be 200 after sharing but we test that in a resource-sharing test class
517+
} else {
518+
// User Fish has FF full access, and has "odfe" backend role which is one of Alice's backend role, so
519+
// Fish should be able to update workflows created by Alice. But the workflow's backend role should
520+
// not be replaced as Fish's backend roles.
521+
Response updateResponse = updateWorkflow(fishClient, workflowId, template);
522+
assertEquals(RestStatus.CREATED, TestHelpers.restStatus(updateResponse));
523+
}
478524
}
479525

480526
public void testUpdateWorkflowWithNoFFAccess() throws Exception {

0 commit comments

Comments
 (0)