Skip to content

Commit 7da9999

Browse files
tvernumtvernum
committed
Switch XPackSecurity user to have own role
1 parent 46614de commit 7da9999

3 files changed

Lines changed: 55 additions & 18 deletions

File tree

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/XPackSecurityUser.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@
66
*/
77
package org.elasticsearch.xpack.core.security.user;
88

9+
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
10+
import org.elasticsearch.xpack.core.security.support.MetadataUtils;
11+
12+
import java.util.Map;
13+
914
/**
1015
* internal user that manages xpack security. Has all cluster/indices permissions.
1116
*/
@@ -14,6 +19,17 @@ public class XPackSecurityUser extends User {
1419
public static final String NAME = UsernamesField.XPACK_SECURITY_NAME;
1520
public static final XPackSecurityUser INSTANCE = new XPackSecurityUser();
1621
private static final String ROLE_NAME = UsernamesField.XPACK_SECURITY_ROLE;
22+
public static final RoleDescriptor ROLE_DESCRIPTOR = new RoleDescriptor(
23+
ROLE_NAME,
24+
new String[] { "all" },
25+
new RoleDescriptor.IndicesPrivileges[] {
26+
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").allowRestrictedIndices(true).build() },
27+
null,
28+
null,
29+
new String[] { "*" },
30+
MetadataUtils.DEFAULT_RESERVED_METADATA,
31+
Map.of()
32+
);
1733

1834
private XPackSecurityUser() {
1935
super(NAME, ROLE_NAME);

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ public class CompositeRolesStore {
100100
private final AtomicLong numInvalidation = new AtomicLong();
101101
private final RoleDescriptorStore roleReferenceResolver;
102102
private final Role superuserRole;
103+
private final Role xpackSecurityRole;
103104
private final Role xpackUserRole;
104105
private final Role asyncSearchUserRole;
105106
private final Automaton restrictedIndicesAutomaton;
@@ -149,6 +150,7 @@ public void providersChanged() {
149150
this.restrictedIndicesAutomaton = resolver.getSystemNameAutomaton();
150151
this.superuserRole = Role.builder(ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR, fieldPermissionsCache, restrictedIndicesAutomaton)
151152
.build();
153+
xpackSecurityRole = Role.builder(XPackSecurityUser.ROLE_DESCRIPTOR, fieldPermissionsCache, restrictedIndicesAutomaton).build();
152154
xpackUserRole = Role.builder(XPackUser.ROLE_DESCRIPTOR, fieldPermissionsCache, restrictedIndicesAutomaton).build();
153155
asyncSearchUserRole = Role.builder(AsyncSearchUser.ROLE_DESCRIPTOR, fieldPermissionsCache, restrictedIndicesAutomaton).build();
154156

@@ -211,11 +213,14 @@ public void getRole(Subject subject, ActionListener<Role> roleActionListener) {
211213
}, roleActionListener::onFailure));
212214
}
213215

214-
private Role tryGetRoleForInternalUser(Subject subject) {
216+
// Accessible by tests
217+
Role tryGetRoleForInternalUser(Subject subject) {
215218
// we need to special case the internal users in this method, if we apply the anonymous roles to every user including these system
216219
// user accounts then we run into the chance of a deadlock because then we need to get a role that we may be trying to get as the
217-
// internal user. The SystemUser is special cased as it has special privileges to execute internal actions and should never be
218-
// passed into this method. The XPackSecurityUser has the Superuser role and we can simply return that
220+
// internal user.
221+
// The SystemUser is special cased as it has special privileges to execute internal actions and should never be passed into this
222+
// method.
223+
// The other internal users have directly assigned roles that are handled with special cases here
219224
final User user = subject.getUser();
220225
if (SystemUser.is(user)) {
221226
throw new IllegalArgumentException(
@@ -227,7 +232,7 @@ private Role tryGetRoleForInternalUser(Subject subject) {
227232
return xpackUserRole;
228233
}
229234
if (XPackSecurityUser.is(user)) {
230-
return superuserRole;
235+
return xpackSecurityRole;
231236
}
232237
if (AsyncSearchUser.is(user)) {
233238
return asyncSearchUserRole;

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java‎

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@
7979
import org.elasticsearch.xpack.core.security.user.AsyncSearchUser;
8080
import org.elasticsearch.xpack.core.security.user.SystemUser;
8181
import org.elasticsearch.xpack.core.security.user.User;
82+
import org.elasticsearch.xpack.core.security.user.XPackSecurityUser;
8283
import org.elasticsearch.xpack.core.security.user.XPackUser;
8384
import org.elasticsearch.xpack.core.watcher.transport.actions.get.GetWatchAction;
8485
import org.elasticsearch.xpack.security.Security;
@@ -127,6 +128,7 @@
127128
import static org.hamcrest.Matchers.hasItem;
128129
import static org.hamcrest.Matchers.hasSize;
129130
import static org.hamcrest.Matchers.is;
131+
import static org.hamcrest.Matchers.notNullValue;
130132
import static org.hamcrest.Matchers.nullValue;
131133
import static org.mockito.ArgumentMatchers.any;
132134
import static org.mockito.ArgumentMatchers.anyCollection;
@@ -1697,6 +1699,21 @@ private Authentication createAuthentication() {
16971699
);
16981700
}
16991701

1702+
public void testXPackSecurityUserCanAccessAnyIndex() {
1703+
for (String action : Arrays.asList(GetAction.NAME, DeleteAction.NAME, SearchAction.NAME, IndexAction.NAME)) {
1704+
Predicate<IndexAbstraction> predicate = getXPackSecurityRole().indices().allowedIndicesMatcher(action);
1705+
1706+
IndexAbstraction index = mockIndexAbstraction(randomAlphaOfLengthBetween(3, 12));
1707+
assertThat(predicate.test(index), Matchers.is(true));
1708+
1709+
index = mockIndexAbstraction("." + randomAlphaOfLengthBetween(3, 12));
1710+
assertThat(predicate.test(index), Matchers.is(true));
1711+
1712+
index = mockIndexAbstraction(".security-" + randomIntBetween(1, 16));
1713+
assertThat(predicate.test(index), Matchers.is(true));
1714+
}
1715+
}
1716+
17001717
public void testXPackUserCanAccessNonRestrictedIndices() {
17011718
CharacterRunAutomaton restrictedAutomaton = new CharacterRunAutomaton(TestRestrictedIndices.RESTRICTED_INDICES_AUTOMATON);
17021719
for (String action : Arrays.asList(GetAction.NAME, DeleteAction.NAME, SearchAction.NAME, IndexAction.NAME)) {
@@ -1802,23 +1819,19 @@ private void getRoleForRoleNames(CompositeRolesStore rolesStore, Collection<Stri
18021819
rolesStore.getRole(subject, listener);
18031820
}
18041821

1822+
private Role getXPackSecurityRole() {
1823+
return getInternalUserRole(XPackSecurityUser.INSTANCE);
1824+
}
1825+
18051826
private Role getXPackUserRole() {
1806-
CompositeRolesStore compositeRolesStore = buildCompositeRolesStore(
1807-
SECURITY_ENABLED_SETTINGS,
1808-
null,
1809-
null,
1810-
null,
1811-
null,
1812-
null,
1813-
null,
1814-
null,
1815-
null,
1816-
null
1817-
);
1818-
return compositeRolesStore.getXpackUserRole();
1827+
return getInternalUserRole(XPackUser.INSTANCE);
18191828
}
18201829

18211830
private Role getAsyncSearchUserRole() {
1831+
return getInternalUserRole(AsyncSearchUser.INSTANCE);
1832+
}
1833+
1834+
private Role getInternalUserRole(User internalUser) {
18221835
CompositeRolesStore compositeRolesStore = buildCompositeRolesStore(
18231836
SECURITY_ENABLED_SETTINGS,
18241837
null,
@@ -1831,7 +1844,10 @@ private Role getAsyncSearchUserRole() {
18311844
null,
18321845
null
18331846
);
1834-
return compositeRolesStore.getAsyncSearchUserRole();
1847+
final Subject subject = new Subject(internalUser, new RealmRef("__attach", "__attach", randomAlphaOfLength(8)));
1848+
final Role role = compositeRolesStore.tryGetRoleForInternalUser(subject);
1849+
assertThat("Role for " + subject, role, notNullValue());
1850+
return role;
18351851
}
18361852

18371853
private CompositeRolesStore buildCompositeRolesStore(

0 commit comments

Comments
 (0)