Skip to content

Commit f4b720e

Browse files
App permissions with action patterns do not retrieve privileges (#85455)
In order for file realm users, with application privileges, to continue to work (authenticate) when the `.security` index is unavailable, Security must avoid loading the application privileges from the .security index (which is unavailable, hence error out). Roles (defined inside the `roles.yml` file, not in the index) that must work when the `.security` index is unavailable must use inlined action patterns for application permisions, rather than privilege names. Related #85312
1 parent dcada70 commit f4b720e

File tree

8 files changed

+133
-11
lines changed

8 files changed

+133
-11
lines changed

docs/changelog/85455.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 85455
2+
summary: App permissions with action patterns do not retrieve privileges
3+
area: Authorization
4+
type: enhancement
5+
issues: []

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/privilege/PutPrivilegesRequest.java

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,8 @@ public ActionRequestValidationException validate() {
6363
validationException = addValidationError("Application privileges must have at least one action", validationException);
6464
}
6565
for (String action : privilege.getActions()) {
66-
if (action.indexOf('/') == -1 && action.indexOf('*') == -1 && action.indexOf(':') == -1) {
67-
validationException = addValidationError(
68-
"action [" + action + "] must contain one of [ '/' , '*' , ':' ]",
69-
validationException
70-
);
71-
}
7266
try {
73-
ApplicationPrivilege.validatePrivilegeOrActionName(action);
67+
ApplicationPrivilege.validateActionName(action);
7468
} catch (IllegalArgumentException e) {
7569
validationException = addValidationError(e.getMessage(), validationException);
7670
}

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,17 +163,32 @@ public static void validatePrivilegeName(String name) {
163163
}
164164
}
165165

166-
private static boolean isValidPrivilegeName(String name) {
166+
public static boolean isValidPrivilegeName(String name) {
167167
return VALID_NAME.matcher(name).matches();
168168
}
169169

170+
public static void validateActionName(String action) {
171+
if (action.indexOf('/') == -1 && action.indexOf('*') == -1 && action.indexOf(':') == -1) {
172+
throw new IllegalArgumentException("action [" + action + "] must contain one of [ '/' , '*' , ':' ]");
173+
}
174+
if (false == isValidPrivilegeOrActionName(action)) {
175+
throw new IllegalArgumentException(
176+
"Application privilege names and actions must match the pattern "
177+
+ VALID_NAME_OR_ACTION.pattern()
178+
+ " (found '"
179+
+ action
180+
+ "')"
181+
);
182+
}
183+
}
184+
170185
/**
171186
* Validate that the provided name is a valid privilege name or action name, and throws an exception otherwise
172187
*
173188
* @throws IllegalArgumentException if the name is not valid
174189
*/
175190
public static void validatePrivilegeOrActionName(String name) {
176-
if (VALID_NAME_OR_ACTION.matcher(name).matches() == false) {
191+
if (isValidPrivilegeOrActionName(name) == false) {
177192
throw new IllegalArgumentException(
178193
"Application privilege names and actions must match the pattern "
179194
+ VALID_NAME_OR_ACTION.pattern()
@@ -184,6 +199,10 @@ public static void validatePrivilegeOrActionName(String name) {
184199
}
185200
}
186201

202+
private static boolean isValidPrivilegeOrActionName(String name) {
203+
return VALID_NAME_OR_ACTION.matcher(name).matches();
204+
}
205+
187206
/**
188207
* Finds or creates a collection of application privileges with the provided names.
189208
* If application is a wildcard, it will be expanded to all matching application names in {@code stored}

x-pack/plugin/security/qa/smoke-test-all-realms/build.gradle

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import org.elasticsearch.gradle.Version
2-
31
/*
42
* This QA test is intended to smoke test all security realms with minimal dependencies.
53
* That is, it makes sure a node that has every realm configured can start, and tests those realms that can be tested without needing external services.
@@ -92,4 +90,5 @@ testClusters.matching { it.name == 'javaRestTest' }.configureEach {
9290
rolesFile file('src/javaRestTest/resources/roles.yml')
9391
user username: "admin_user", password: "admin-password"
9492
user username: "security_test_user", password: "security-test-password", role: "security_test_role"
93+
user username: "index_and_app_user", password: "security-test-password", role: "all_index_privileges,all_application_privileges"
9594
}

x-pack/plugin/security/qa/smoke-test-all-realms/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/FileRealmAuthIT.java

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,17 @@
77

88
package org.elasticsearch.xpack.security.authc;
99

10+
import org.apache.http.client.methods.HttpPost;
11+
import org.elasticsearch.client.Request;
1012
import org.elasticsearch.client.RequestOptions;
13+
import org.elasticsearch.client.WarningsHandler;
1114
import org.elasticsearch.common.settings.SecureString;
1215
import org.elasticsearch.xpack.core.security.authc.Authentication;
1316
import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
17+
import org.elasticsearch.xpack.core.security.test.TestRestrictedIndices;
1418

1519
import java.io.IOException;
20+
import java.util.List;
1621
import java.util.Map;
1722

1823
/**
@@ -22,6 +27,7 @@ public class FileRealmAuthIT extends SecurityRealmSmokeTestCase {
2227

2328
// Declared in build.gradle
2429
private static final String USERNAME = "security_test_user";
30+
private static final String ANOTHER_USERNAME = "index_and_app_user";
2531
private static final SecureString PASSWORD = new SecureString("security-test-password".toCharArray());
2632
private static final String ROLE_NAME = "security_test_role";
2733

@@ -36,4 +42,45 @@ public void testAuthenticationUsingFileRealm() throws IOException {
3642
assertNoApiKeyInfo(authenticate, Authentication.AuthenticationType.REALM);
3743
}
3844

45+
public void testAuthenticationUsingFileRealmAndNoSecurityIndex() throws IOException {
46+
Map<String, Object> authenticate = super.authenticate(
47+
RequestOptions.DEFAULT.toBuilder()
48+
.addHeader("Authorization", UsernamePasswordToken.basicAuthHeaderValue(ANOTHER_USERNAME, PASSWORD))
49+
);
50+
51+
try {
52+
// create user to ensure the .security-7 index exists
53+
createUser("dummy", new SecureString("longpassword".toCharArray()), List.of("whatever"));
54+
// close the .security-7 to simulate making it unavailable
55+
Request closeRequest = new Request(HttpPost.METHOD_NAME, TestRestrictedIndices.INTERNAL_SECURITY_MAIN_INDEX_7 + "/_close");
56+
closeRequest.setOptions(
57+
RequestOptions.DEFAULT.toBuilder()
58+
.addHeader("Authorization", UsernamePasswordToken.basicAuthHeaderValue(ANOTHER_USERNAME, PASSWORD))
59+
.setWarningsHandler(WarningsHandler.PERMISSIVE)
60+
);
61+
assertOK(client().performRequest(closeRequest));
62+
// clear the authentication cache
63+
Request clearCachesRequest = new Request(HttpPost.METHOD_NAME, "_security/realm/*/_clear_cache");
64+
clearCachesRequest.setOptions(
65+
RequestOptions.DEFAULT.toBuilder()
66+
.addHeader("Authorization", UsernamePasswordToken.basicAuthHeaderValue(ANOTHER_USERNAME, PASSWORD))
67+
);
68+
assertOK(client().performRequest(clearCachesRequest));
69+
70+
// file-realm authentication still works when cache is cleared and .security-7 is out
71+
assertUsername(authenticate, ANOTHER_USERNAME);
72+
assertRealm(authenticate, "file", "file0");
73+
assertRoles(authenticate, "all_index_privileges", "all_application_privileges");
74+
assertNoApiKeyInfo(authenticate, Authentication.AuthenticationType.REALM);
75+
} finally {
76+
Request openRequest = new Request(HttpPost.METHOD_NAME, TestRestrictedIndices.INTERNAL_SECURITY_MAIN_INDEX_7 + "/_open");
77+
openRequest.setOptions(
78+
RequestOptions.DEFAULT.toBuilder()
79+
.addHeader("Authorization", UsernamePasswordToken.basicAuthHeaderValue(ANOTHER_USERNAME, PASSWORD))
80+
.setWarningsHandler(WarningsHandler.PERMISSIVE)
81+
);
82+
assertOK(client().performRequest(openRequest));
83+
deleteUser("dummy");
84+
}
85+
}
3986
}

x-pack/plugin/security/qa/smoke-test-all-realms/src/javaRestTest/resources/roles.yml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,17 @@ security_test_role:
66
indices:
77
- names: [ "index_allowed" ]
88
privileges: [ "read", "write", "create_index" ]
9+
10+
all_index_privileges:
11+
cluster:
12+
- "cluster:admin/xpack/security/realm/cache/clear"
13+
indices:
14+
- names: [ '*' ]
15+
privileges: [ 'all' ]
16+
allow_restricted_indices: true
17+
18+
all_application_privileges:
19+
applications:
20+
- application: "*"
21+
privileges: [ "*" ]
22+
resources: [ "*" ]

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import org.elasticsearch.xpack.core.security.action.privilege.ClearPrivilegesCacheAction;
4646
import org.elasticsearch.xpack.core.security.action.privilege.ClearPrivilegesCacheRequest;
4747
import org.elasticsearch.xpack.core.security.action.privilege.ClearPrivilegesCacheResponse;
48+
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege;
4849
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;
4950
import org.elasticsearch.xpack.security.support.CacheInvalidatorRegistry;
5051
import org.elasticsearch.xpack.security.support.LockingAtomicCounter;
@@ -130,6 +131,11 @@ public void getPrivileges(
130131
Collection<String> names,
131132
ActionListener<Collection<ApplicationPrivilegeDescriptor>> listener
132133
) {
134+
if (false == isEmpty(names) && names.stream().noneMatch(ApplicationPrivilege::isValidPrivilegeName)) {
135+
logger.debug("no concrete privilege, only action patterns [{}], returning no application privilege descriptors", names);
136+
listener.onResponse(Collections.emptySet());
137+
return;
138+
}
133139

134140
final Set<String> applicationNamesCacheKey = (isEmpty(applications) || applications.contains("*"))
135141
? Set.of("*")

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.elasticsearch.xcontent.XContentBuilder;
3838
import org.elasticsearch.xcontent.XContentType;
3939
import org.elasticsearch.xpack.core.security.action.privilege.ClearPrivilegesCacheRequest;
40+
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege;
4041
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;
4142
import org.elasticsearch.xpack.core.security.test.TestRestrictedIndices;
4243
import org.elasticsearch.xpack.security.support.CacheInvalidatorRegistry;
@@ -81,6 +82,7 @@
8182
import static org.hamcrest.Matchers.not;
8283
import static org.mockito.ArgumentMatchers.any;
8384
import static org.mockito.Mockito.mock;
85+
import static org.mockito.Mockito.verifyNoInteractions;
8486
import static org.mockito.Mockito.when;
8587

8688
public class NativePrivilegeStoreTests extends ESTestCase {
@@ -713,6 +715,42 @@ public void testPutPrivileges() throws Exception {
713715
assertThat(map.get("app2"), contains("all"));
714716
}
715717

718+
public void testRetrieveActionNamePatternsInsteadOfPrivileges() throws Exception {
719+
// test disabling caching
720+
final PlainActionFuture<Collection<ApplicationPrivilegeDescriptor>> future = new PlainActionFuture<>();
721+
for (List<String> applications : List.<List<String>>of(
722+
List.of("myapp"),
723+
List.of("myapp*"),
724+
List.of("myapp", "myapp*"),
725+
List.of(),
726+
List.of("*"),
727+
List.of("myapp-2", "*")
728+
)) {
729+
Collection<String> actions = randomList(1, 4, () -> {
730+
String actionName = randomAlphaOfLengthBetween(0, 3) + randomFrom("*", "/", ":") + randomAlphaOfLengthBetween(0, 3)
731+
+ randomFrom("*", "/", ":", "");
732+
ApplicationPrivilege.validateActionName(actionName);
733+
return actionName;
734+
});
735+
Client mockClient = mock(Client.class);
736+
SecurityIndexManager mockSecurityIndexManager = mock(SecurityIndexManager.class);
737+
Settings settings = randomFrom(
738+
Settings.builder().put("xpack.security.authz.store.privileges.cache.ttl", 0).build(),
739+
Settings.EMPTY
740+
);
741+
NativePrivilegeStore store1 = new NativePrivilegeStore(
742+
settings,
743+
mockClient,
744+
mockSecurityIndexManager,
745+
new CacheInvalidatorRegistry()
746+
);
747+
store1.getPrivileges(applications, actions, future);
748+
assertResult(emptyList(), future);
749+
verifyNoInteractions(mockClient);
750+
verifyNoInteractions(mockSecurityIndexManager);
751+
}
752+
}
753+
716754
public void testDeletePrivileges() throws Exception {
717755
final List<String> privilegeNames = Arrays.asList("p1", "p2", "p3");
718756

0 commit comments

Comments
 (0)