Skip to content

Commit cdd2d58

Browse files
authored
Use 'should' clause instead of 'filter' when querying native privileges (#47019)
When we added support for wildcard application names, we started to build the prefix query along with the term query but we used 'filter' clause instead of 'should', so this would not fetch the correct application privilege descriptor thereby failing the _has_privilege checks. This commit changes the clause to use should and with minimum_should_match as 1.
1 parent 38b847a commit cdd2d58

File tree

3 files changed

+89
-3
lines changed

3 files changed

+89
-3
lines changed

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

+3-2
Original file line numberDiff line numberDiff line change
@@ -177,12 +177,13 @@ private QueryBuilder getApplicationNameQuery(Collection<String> applications) {
177177
}
178178
final BoolQueryBuilder boolQuery = QueryBuilders.boolQuery();
179179
if (termsQuery != null) {
180-
boolQuery.filter(termsQuery);
180+
boolQuery.should(termsQuery);
181181
}
182182
for (String wildcard : wildcardNames) {
183183
final String prefix = wildcard.substring(0, wildcard.length() - 1);
184-
boolQuery.filter(QueryBuilders.prefixQuery(APPLICATION.getPreferredName(), prefix));
184+
boolQuery.should(QueryBuilders.prefixQuery(APPLICATION.getPreferredName(), prefix));
185185
}
186+
boolQuery.minimumShouldMatch(1);
186187
return boolQuery;
187188
}
188189

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ public void testGetPrivilegesByWildcardApplicationName() throws Exception {
189189
assertThat(request.indices(), arrayContaining(RestrictedIndicesNames.SECURITY_MAIN_ALIAS));
190190

191191
final String query = Strings.toString(request.source().query());
192-
assertThat(query, containsString("{\"bool\":{\"filter\":[{\"terms\":{\"application\":[\"yourapp\"]"));
192+
assertThat(query, containsString("{\"bool\":{\"should\":[{\"terms\":{\"application\":[\"yourapp\"]"));
193193
assertThat(query, containsString("{\"prefix\":{\"application\":{\"value\":\"myapp-\""));
194194
assertThat(query, containsString("{\"term\":{\"type\":{\"value\":\"application-privilege\""));
195195

x-pack/plugin/src/test/resources/rest-api-spec/test/privileges/20_has_application_privs.yml

+85
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,27 @@ setup:
109109
]
110110
}
111111
112+
- do:
113+
security.put_role:
114+
name: "role_containing_wildcard_app_name_and_plain_app_name"
115+
body: >
116+
{
117+
"cluster": [],
118+
"indices": [],
119+
"applications": [
120+
{
121+
"application": "myapp",
122+
"privileges": ["user"],
123+
"resources": ["*"]
124+
},
125+
{
126+
"application": "yourapp-*",
127+
"privileges": ["read"],
128+
"resources": ["*"]
129+
}
130+
]
131+
}
132+
112133
# And a user for each role
113134
- do:
114135
security.put_user:
@@ -134,6 +155,14 @@ setup:
134155
"password": "p@ssw0rd",
135156
"roles" : [ "yourapp_read_config" ]
136157
}
158+
- do:
159+
security.put_user:
160+
username: "myapp_yourapp_wildard_role_user"
161+
body: >
162+
{
163+
"password": "p@ssw0rd",
164+
"roles" : [ "role_containing_wildcard_app_name_and_plain_app_name" ]
165+
}
137166
138167
---
139168
teardown:
@@ -168,6 +197,11 @@ teardown:
168197
username: "your_read"
169198
ignore: 404
170199

200+
- do:
201+
security.delete_user:
202+
username: "myapp_yourapp_wildard_role_user"
203+
ignore: 404
204+
171205
- do:
172206
security.delete_role:
173207
name: "myapp_engineering_read"
@@ -182,6 +216,12 @@ teardown:
182216
security.delete_role:
183217
name: "yourapp_read_config"
184218
ignore: 404
219+
220+
- do:
221+
security.delete_role:
222+
name: "role_containing_wildcard_app_name_and_plain_app_name"
223+
ignore: 404
224+
185225
---
186226
"Test has_privileges with application-privileges":
187227
- do:
@@ -291,3 +331,48 @@ teardown:
291331
}
292332
}
293333
} }
334+
335+
- do:
336+
headers: { Authorization: "Basic bXlhcHBfeW91cmFwcF93aWxkYXJkX3JvbGVfdXNlcjpwQHNzdzByZA==" } # myapp_yourapp_wildard_role_user
337+
security.has_privileges:
338+
user: null
339+
body: >
340+
{
341+
"application": [
342+
{
343+
"application" : "myapp",
344+
"resources" : [ "*" ],
345+
"privileges" : [ "action:login" ]
346+
},
347+
{
348+
"application" : "yourapp-v1",
349+
"resources" : [ "*" ],
350+
"privileges" : [ "read" ]
351+
},
352+
{
353+
"application" : "yourapp-v2",
354+
"resources" : [ "*" ],
355+
"privileges" : [ "read" ]
356+
}
357+
]
358+
}
359+
360+
- match: { "username" : "myapp_yourapp_wildard_role_user" }
361+
- match: { "has_all_requested" : true }
362+
- match: { "application" : {
363+
"myapp" : {
364+
"*" : {
365+
"action:login" : true
366+
}
367+
},
368+
"yourapp-v1": {
369+
"*": {
370+
"read": true
371+
}
372+
},
373+
"yourapp-v2": {
374+
"*": {
375+
"read": true
376+
}
377+
}
378+
} }

0 commit comments

Comments
 (0)