Disabling users includes realm in same-user validation (#86473)

A validation check prevents users from disabling themselves. The check is incorrect since it does not factor in realms. Users can have overlapping usernames across realms, and should be able to disable same-named users in other realms, authorization provided. This PR tweaks the validation check to account for the source realm.
elastic · May 6, 2022 · 313893d · 313893d
1 parent e30a069
commit 313893d
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 14 deletions.
diff --git a/docs/changelog/86473.yaml b/docs/changelog/86473.yaml
@@ -0,0 +1,5 @@
+pr: 86473
+summary: An authorized user can disable a user with same name but different realm
+area: Authentication
+type: bug
+issues: []
diff --git a/...src/main/java/org/elasticsearch/xpack/security/action/user/TransportSetEnabledAction.java b/...src/main/java/org/elasticsearch/xpack/security/action/user/TransportSetEnabledAction.java
@@ -17,8 +17,10 @@
 import org.elasticsearch.xpack.core.security.SecurityContext;
 import org.elasticsearch.xpack.core.security.action.user.SetEnabledAction;
 import org.elasticsearch.xpack.core.security.action.user.SetEnabledRequest;
+import org.elasticsearch.xpack.core.security.authc.esnative.NativeRealmSettings;
 import org.elasticsearch.xpack.core.security.user.AnonymousUser;
 import org.elasticsearch.xpack.security.authc.esnative.NativeUsersStore;
+import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm;
 
 /**
  * Transport action that handles setting a native or reserved user to enabled
@@ -47,7 +49,7 @@ public TransportSetEnabledAction(
     protected void doExecute(Task task, SetEnabledRequest request, ActionListener<ActionResponse.Empty> listener) {
         final String username = request.username();
         // make sure the user is not disabling themselves
-        if (securityContext.getUser().principal().equals(request.username())) {
+        if (isSameUserRequest(request)) {
             listener.onFailure(new IllegalArgumentException("users may not update the enabled status of their own account"));
             return;
         } else if (AnonymousUser.isAnonymousUsername(username, settings)) {
@@ -62,4 +64,13 @@ protected void doExecute(Task task, SetEnabledRequest request, ActionListener<Ac
             listener.delegateFailure((l, v) -> l.onResponse(ActionResponse.Empty.INSTANCE))
         );
     }
+
+    private boolean isSameUserRequest(SetEnabledRequest request) {
+        final var effectiveSubject = securityContext.getAuthentication().getEffectiveSubject();
+        final var realmType = effectiveSubject.getRealm().getType();
+        // Only native or reserved realm users can be disabled via the API. If the realm of the effective subject is neither,
+        // the target must be a different user
+        return (ReservedRealm.TYPE.equals(realmType) || NativeRealmSettings.TYPE.equals(realmType))
+            && effectiveSubject.getUser().principal().equals(request.username());
+    }
 }
diff --git a/...est/java/org/elasticsearch/xpack/security/action/user/TransportSetEnabledActionTests.java b/...est/java/org/elasticsearch/xpack/security/action/user/TransportSetEnabledActionTests.java
@@ -22,13 +22,16 @@
 import org.elasticsearch.xpack.core.security.action.user.SetEnabledRequest;
 import org.elasticsearch.xpack.core.security.authc.Authentication;
 import org.elasticsearch.xpack.core.security.authc.AuthenticationTestHelper;
+import org.elasticsearch.xpack.core.security.authc.Subject;
+import org.elasticsearch.xpack.core.security.authc.esnative.NativeRealmSettings;
 import org.elasticsearch.xpack.core.security.authc.support.AuthenticationContextSerializer;
 import org.elasticsearch.xpack.core.security.user.AnonymousUser;
 import org.elasticsearch.xpack.core.security.user.ElasticUser;
 import org.elasticsearch.xpack.core.security.user.KibanaUser;
 import org.elasticsearch.xpack.core.security.user.SystemUser;
 import org.elasticsearch.xpack.core.security.user.User;
 import org.elasticsearch.xpack.security.authc.esnative.NativeUsersStore;
+import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm;
 
 import java.io.IOException;
 import java.util.Collections;
@@ -42,6 +45,7 @@
 import static org.hamcrest.Matchers.nullValue;
 import static org.hamcrest.Matchers.sameInstance;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
@@ -60,8 +64,9 @@ public void testAnonymousUser() throws Exception {
         ThreadPool threadPool = mock(ThreadPool.class);
         ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
         when(threadPool.getThreadContext()).thenReturn(threadContext);
-        Authentication authentication = mock(Authentication.class);
-        when(authentication.getUser()).thenReturn(user);
+        Authentication authentication = mock(Authentication.class, RETURNS_DEEP_STUBS);
+        when(authentication.getEffectiveSubject().getUser()).thenReturn(user);
+        when(authentication.getEffectiveSubject().getRealm().getType()).thenReturn(NativeRealmSettings.TYPE);
         when(authentication.encode()).thenReturn(randomAlphaOfLength(24)); // just can't be null
         new AuthenticationContextSerializer().writeToContext(authentication, threadContext);
 
@@ -109,21 +114,30 @@ public void onFailure(Exception e) {
     }
 
     public void testValidUser() throws Exception {
-        testValidUser(randomFrom(new ElasticUser(true), new KibanaUser(true), new User("joe"), new User(SystemUser.INSTANCE.principal())));
+        testValidUser(
+            randomFrom(new ElasticUser(true), new KibanaUser(true), new User("joe"), new User(SystemUser.INSTANCE.principal())),
+            defaultAuthentication()
+        );
     }
 
     public void testValidUserWithInternalUsername() throws Exception {
-        testValidUser(new User(AuthenticationTestHelper.randomInternalUsername()));
+        testValidUser(new User(AuthenticationTestHelper.randomInternalUsername()), defaultAuthentication());
     }
 
-    private void testValidUser(User user) throws IOException {
+    public void testUserCanModifySameNameUserFromDifferentRealm() throws Exception {
+        final User user = randomFrom(new ElasticUser(true), new KibanaUser(true), new User("joe"));
+        Authentication authentication = mock(Authentication.class, RETURNS_DEEP_STUBS);
+        when(authentication.getEffectiveSubject().getUser()).thenReturn(user);
+        when(authentication.getEffectiveSubject().getRealm().getType()).thenReturn("other_realm");
+        when(authentication.encode()).thenReturn(randomAlphaOfLength(24)); // just can't be null
+        testValidUser(user, authentication);
+    }
+
+    private void testValidUser(User user, Authentication authentication) throws IOException {
         ThreadPool threadPool = mock(ThreadPool.class);
         ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
         when(threadPool.getThreadContext()).thenReturn(threadContext);
 
-        Authentication authentication = mock(Authentication.class);
-        when(authentication.getUser()).thenReturn(new User("the runner"));
-        when(authentication.encode()).thenReturn(randomAlphaOfLength(24)); // just can't be null
         new AuthenticationContextSerializer().writeToContext(authentication, threadContext);
 
         NativeUsersStore usersStore = mock(NativeUsersStore.class);
@@ -188,10 +202,7 @@ public void testException() throws Exception {
         ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
         when(threadPool.getThreadContext()).thenReturn(threadContext);
 
-        Authentication authentication = mock(Authentication.class);
-        when(authentication.getUser()).thenReturn(new User("the runner"));
-        when(authentication.encode()).thenReturn(randomAlphaOfLength(24)); // just can't be null
-        new AuthenticationContextSerializer().writeToContext(authentication, threadContext);
+        new AuthenticationContextSerializer().writeToContext(defaultAuthentication(), threadContext);
 
         final User user = randomFrom(new ElasticUser(true), new KibanaUser(true), new User("joe"));
         NativeUsersStore usersStore = mock(NativeUsersStore.class);
@@ -259,7 +270,10 @@ public void testUserModifyingThemselves() throws Exception {
         when(threadPool.getThreadContext()).thenReturn(threadContext);
 
         Authentication authentication = mock(Authentication.class);
-        when(authentication.getUser()).thenReturn(user);
+        Subject effectiveSubject = mock(Subject.class, RETURNS_DEEP_STUBS);
+        when(authentication.getEffectiveSubject()).thenReturn(effectiveSubject);
+        when(effectiveSubject.getUser()).thenReturn(user);
+        when(effectiveSubject.getRealm().getType()).thenReturn(randomFrom(NativeRealmSettings.TYPE, ReservedRealm.TYPE));
         when(authentication.encode()).thenReturn(randomAlphaOfLength(24)); // just can't be null
         new AuthenticationContextSerializer().writeToContext(authentication, threadContext);
 
@@ -305,4 +319,12 @@ public void onFailure(Exception e) {
         assertThat(throwableRef.get().getMessage(), containsString("own account"));
         verifyNoMoreInteractions(usersStore);
     }
+
+    private Authentication defaultAuthentication() throws IOException {
+        Authentication authentication = mock(Authentication.class, RETURNS_DEEP_STUBS);
+        when(authentication.getEffectiveSubject().getUser()).thenReturn(new User("the runner"));
+        when(authentication.getEffectiveSubject().getRealm().getType()).thenReturn(NativeRealmSettings.TYPE);
+        when(authentication.encode()).thenReturn(randomAlphaOfLength(24)); // just can't be null
+        return authentication;
+    }
 }