Stop execution once destructive operations check has failed (elastic/elasticsearch#4337)

Otherwise we do return an error, but we also go ahead and open/close/delete the indices anyways.

Original commit: elastic/x-pack-elasticsearch@176eca4cff
This commit is contained in:
Luca Cavanna 2016-12-13 11:22:48 +01:00 committed by GitHub
parent b57c4f6ebe
commit 1c846dd893
5 changed files with 14 additions and 1 deletions

View File

@ -152,6 +152,7 @@ public class SecurityActionFilter extends AbstractComponent implements ActionFil
destructiveOperations.failDestructive(indicesRequest.indices()); destructiveOperations.failDestructive(indicesRequest.indices());
} catch(IllegalArgumentException e) { } catch(IllegalArgumentException e) {
listener.onFailure(e); listener.onFailure(e);
return;
} }
} }

View File

@ -84,6 +84,7 @@ public interface ServerTransportFilter {
destructiveOperations.failDestructive(indicesRequest.indices()); destructiveOperations.failDestructive(indicesRequest.indices());
} catch(IllegalArgumentException e) { } catch(IllegalArgumentException e) {
listener.onFailure(e); listener.onFailure(e);
return;
} }
} }
/* /*

View File

@ -21,25 +21,34 @@ public class DestructiveOperationsTests extends SecurityIntegTestCase {
} }
public void testDeleteIndexDestructiveOperationsRequireName() { public void testDeleteIndexDestructiveOperationsRequireName() {
createIndex("index1");
Settings settings = Settings.builder().put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), true).build(); Settings settings = Settings.builder().put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), true).build();
assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings)); assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
{ {
IllegalArgumentException illegalArgumentException = expectThrows(IllegalArgumentException.class, IllegalArgumentException illegalArgumentException = expectThrows(IllegalArgumentException.class,
() -> client().admin().indices().prepareDelete("*").get()); () -> client().admin().indices().prepareDelete("*").get());
assertEquals("Wildcard expressions or all indices are not allowed", illegalArgumentException.getMessage()); assertEquals("Wildcard expressions or all indices are not allowed", illegalArgumentException.getMessage());
String[] indices = client().admin().indices().prepareGetIndex().setIndices("index1").get().getIndices();
assertEquals(1, indices.length);
assertEquals("index1", indices[0]);
} }
{ {
IllegalArgumentException illegalArgumentException = expectThrows(IllegalArgumentException.class, IllegalArgumentException illegalArgumentException = expectThrows(IllegalArgumentException.class,
() -> client().admin().indices().prepareDelete("*", "-index1").get()); () -> client().admin().indices().prepareDelete("*", "-index1").get());
assertEquals("Wildcard expressions or all indices are not allowed", illegalArgumentException.getMessage()); assertEquals("Wildcard expressions or all indices are not allowed", illegalArgumentException.getMessage());
String[] indices = client().admin().indices().prepareGetIndex().setIndices("index1").get().getIndices();
assertEquals(1, indices.length);
assertEquals("index1", indices[0]);
} }
{ {
IllegalArgumentException illegalArgumentException = expectThrows(IllegalArgumentException.class, IllegalArgumentException illegalArgumentException = expectThrows(IllegalArgumentException.class,
() -> client().admin().indices().prepareDelete("_all").get()); () -> client().admin().indices().prepareDelete("_all").get());
assertEquals("Wildcard expressions or all indices are not allowed", illegalArgumentException.getMessage()); assertEquals("Wildcard expressions or all indices are not allowed", illegalArgumentException.getMessage());
String[] indices = client().admin().indices().prepareGetIndex().setIndices("index1").get().getIndices();
assertEquals(1, indices.length);
assertEquals("index1", indices[0]);
} }
createIndex("index1");
assertAcked(client().admin().indices().prepareDelete("index1")); assertAcked(client().admin().indices().prepareDelete("index1"));
} }

View File

@ -133,6 +133,7 @@ public class SecurityActionFilterTests extends ESTestCase {
filter.apply(task, action, request, listener, chain); filter.apply(task, action, request, listener, chain);
if (failDestructiveOperations) { if (failDestructiveOperations) {
verify(listener).onFailure(isA(IllegalArgumentException.class)); verify(listener).onFailure(isA(IllegalArgumentException.class));
verifyNoMoreInteractions(authzService, chain);
} else { } else {
verify(authzService).authorize(authentication, action, request, Collections.emptyList(), Collections.emptyList()); verify(authzService).authorize(authentication, action, request, Collections.emptyList(), Collections.emptyList());
verify(chain).proceed(eq(task), eq(action), eq(request), isA(ContextPreservingActionListener.class)); verify(chain).proceed(eq(task), eq(action), eq(request), isA(ContextPreservingActionListener.class));

View File

@ -106,6 +106,7 @@ public class ServerTransportFilterTests extends ESTestCase {
filter.inbound(action, request, channel, listener); filter.inbound(action, request, channel, listener);
if (failDestructiveOperations) { if (failDestructiveOperations) {
verify(listener).onFailure(isA(IllegalArgumentException.class)); verify(listener).onFailure(isA(IllegalArgumentException.class));
verifyNoMoreInteractions(authzService);
} else { } else {
verify(authzService).authorize(authentication, action, request, Collections.emptyList(), Collections.emptyList()); verify(authzService).authorize(authentication, action, request, Collections.emptyList(), Collections.emptyList());
} }