diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java index f5175b526be..98dad61ab19 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java @@ -427,11 +427,13 @@ public class AuthenticationService { if (index > 0) { final List smartOrder = new ArrayList<>(orderedRealmList.size()); smartOrder.add(lastSuccess); - for (int i = 1; i < orderedRealmList.size(); i++) { + for (int i = 0; i < orderedRealmList.size(); i++) { if (i != index) { smartOrder.add(orderedRealmList.get(i)); } } + assert smartOrder.size() == orderedRealmList.size() && smartOrder.containsAll(orderedRealmList) + : "Element mismatch between SmartOrder=" + smartOrder + " and DefaultOrder=" + orderedRealmList; return Collections.unmodifiableList(smartOrder); } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java index 955f24d6f26..fcde6aad488 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java @@ -134,6 +134,10 @@ import static org.mockito.Mockito.when; */ public class AuthenticationServiceTests extends ESTestCase { + private static final String SECOND_REALM_NAME = "second_realm"; + private static final String SECOND_REALM_TYPE = "second"; + private static final String FIRST_REALM_NAME = "file_realm"; + private static final String FIRST_REALM_TYPE = "file"; private AuthenticationService service; private TransportMessage message; private RestRequest restRequest; @@ -167,11 +171,11 @@ public class AuthenticationServiceTests extends ESTestCase { threadContext = new ThreadContext(Settings.EMPTY); firstRealm = mock(Realm.class); - when(firstRealm.type()).thenReturn("file"); - when(firstRealm.name()).thenReturn("file_realm"); + when(firstRealm.type()).thenReturn(FIRST_REALM_TYPE); + when(firstRealm.name()).thenReturn(FIRST_REALM_NAME); secondRealm = mock(Realm.class); - when(secondRealm.type()).thenReturn("second"); - when(secondRealm.name()).thenReturn("second_realm"); + when(secondRealm.type()).thenReturn(SECOND_REALM_TYPE); + when(secondRealm.name()).thenReturn(SECOND_REALM_NAME); Settings settings = Settings.builder() .put("path.home", createTempDir()) .put("node.name", "authc_test") @@ -305,26 +309,35 @@ public class AuthenticationServiceTests extends ESTestCase { when(secondRealm.token(threadContext)).thenReturn(token); final String reqId = AuditUtil.getOrGenerateRequestId(threadContext); + // Authenticate against the normal chain. 1st Realm will be checked (and not pass) then 2nd realm will successfully authc final AtomicBoolean completed = new AtomicBoolean(false); service.authenticate("_action", message, (User)null, ActionListener.wrap(result -> { assertThat(result, notNullValue()); assertThat(result.getUser(), is(user)); assertThat(result.getLookedUpBy(), is(nullValue())); assertThat(result.getAuthenticatedBy(), is(notNullValue())); // TODO implement equals + assertThat(result.getAuthenticatedBy().getName(), is(SECOND_REALM_NAME)); + assertThat(result.getAuthenticatedBy().getType(), is(SECOND_REALM_TYPE)); assertThreadContextContainsAuthentication(result); setCompletedToTrue(completed); }, this::logAndFail)); assertTrue(completed.get()); completed.set(false); + // Authenticate against the smart chain. + // "SecondRealm" will be at the top of the list and will successfully authc. + // "FirstRealm" will not be used service.authenticate("_action", message, (User)null, ActionListener.wrap(result -> { assertThat(result, notNullValue()); assertThat(result.getUser(), is(user)); assertThat(result.getLookedUpBy(), is(nullValue())); assertThat(result.getAuthenticatedBy(), is(notNullValue())); // TODO implement equals + assertThat(result.getAuthenticatedBy().getName(), is(SECOND_REALM_NAME)); + assertThat(result.getAuthenticatedBy().getType(), is(SECOND_REALM_TYPE)); assertThreadContextContainsAuthentication(result); setCompletedToTrue(completed); }, this::logAndFail)); + verify(auditTrail).authenticationFailed(reqId, firstRealm.name(), token, "_action", message); verify(auditTrail, times(2)).authenticationSuccess(reqId, secondRealm.name(), user, "_action", message); verify(firstRealm, times(2)).name(); // used above one time @@ -337,6 +350,30 @@ public class AuthenticationServiceTests extends ESTestCase { verify(firstRealm).authenticate(eq(token), any(ActionListener.class)); verify(secondRealm, times(2)).authenticate(eq(token), any(ActionListener.class)); verifyNoMoreInteractions(auditTrail, firstRealm, secondRealm); + + // Now assume some change in the backend system so that 2nd realm no longer has the user, but the 1st realm does. + mockAuthenticate(secondRealm, token, null); + mockAuthenticate(firstRealm, token, user); + + completed.set(false); + // This will authenticate against the smart chain. + // "SecondRealm" will be at the top of the list but will no longer authenticate the user. + // Then "FirstRealm" will be checked. + service.authenticate("_action", message, (User)null, ActionListener.wrap(result -> { + assertThat(result, notNullValue()); + assertThat(result.getUser(), is(user)); + assertThat(result.getLookedUpBy(), is(nullValue())); + assertThat(result.getAuthenticatedBy(), is(notNullValue())); + assertThat(result.getAuthenticatedBy().getName(), is(FIRST_REALM_NAME)); + assertThat(result.getAuthenticatedBy().getType(), is(FIRST_REALM_TYPE)); + assertThreadContextContainsAuthentication(result); + setCompletedToTrue(completed); + }, this::logAndFail)); + + verify(auditTrail, times(1)).authenticationFailed(reqId, SECOND_REALM_NAME, token, "_action", message); + verify(auditTrail, times(1)).authenticationSuccess(reqId, FIRST_REALM_NAME, user, "_action", message); + verify(secondRealm, times(3)).authenticate(eq(token), any(ActionListener.class)); // 2 from above + 1 more + verify(firstRealm, times(2)).authenticate(eq(token), any(ActionListener.class)); // 1 from above + 1 more } public void testCacheClearOnSecurityIndexChange() {