Use 'should' clause instead of 'filter' when querying native privileges (#47019) (#47271)

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.
This commit is contained in:
Yogesh Gaikwad 2019-09-30 14:14:52 +10:00 committed by GitHub
parent cec2ff5ef4
commit 2be351c5d0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 89 additions and 3 deletions

View File

@ -179,12 +179,13 @@ public class NativePrivilegeStore {
}
final BoolQueryBuilder boolQuery = QueryBuilders.boolQuery();
if (termsQuery != null) {
boolQuery.filter(termsQuery);
boolQuery.should(termsQuery);
}
for (String wildcard : wildcardNames) {
final String prefix = wildcard.substring(0, wildcard.length() - 1);
boolQuery.filter(QueryBuilders.prefixQuery(APPLICATION.getPreferredName(), prefix));
boolQuery.should(QueryBuilders.prefixQuery(APPLICATION.getPreferredName(), prefix));
}
boolQuery.minimumShouldMatch(1);
return boolQuery;
}

View File

@ -191,7 +191,7 @@ public class NativePrivilegeStoreTests extends ESTestCase {
assertThat(request.indices(), arrayContaining(RestrictedIndicesNames.SECURITY_MAIN_ALIAS));
final String query = Strings.toString(request.source().query());
assertThat(query, containsString("{\"bool\":{\"filter\":[{\"terms\":{\"application\":[\"yourapp\"]"));
assertThat(query, containsString("{\"bool\":{\"should\":[{\"terms\":{\"application\":[\"yourapp\"]"));
assertThat(query, containsString("{\"prefix\":{\"application\":{\"value\":\"myapp-\""));
assertThat(query, containsString("{\"term\":{\"type\":{\"value\":\"application-privilege\""));

View File

@ -109,6 +109,27 @@ setup:
]
}
- do:
security.put_role:
name: "role_containing_wildcard_app_name_and_plain_app_name"
body: >
{
"cluster": [],
"indices": [],
"applications": [
{
"application": "myapp",
"privileges": ["user"],
"resources": ["*"]
},
{
"application": "yourapp-*",
"privileges": ["read"],
"resources": ["*"]
}
]
}
# And a user for each role
- do:
security.put_user:
@ -134,6 +155,14 @@ setup:
"password": "p@ssw0rd",
"roles" : [ "yourapp_read_config" ]
}
- do:
security.put_user:
username: "myapp_yourapp_wildard_role_user"
body: >
{
"password": "p@ssw0rd",
"roles" : [ "role_containing_wildcard_app_name_and_plain_app_name" ]
}
---
teardown:
@ -168,6 +197,11 @@ teardown:
username: "your_read"
ignore: 404
- do:
security.delete_user:
username: "myapp_yourapp_wildard_role_user"
ignore: 404
- do:
security.delete_role:
name: "myapp_engineering_read"
@ -182,6 +216,12 @@ teardown:
security.delete_role:
name: "yourapp_read_config"
ignore: 404
- do:
security.delete_role:
name: "role_containing_wildcard_app_name_and_plain_app_name"
ignore: 404
---
"Test has_privileges with application-privileges":
- do:
@ -291,3 +331,48 @@ teardown:
}
}
} }
- do:
headers: { Authorization: "Basic bXlhcHBfeW91cmFwcF93aWxkYXJkX3JvbGVfdXNlcjpwQHNzdzByZA==" } # myapp_yourapp_wildard_role_user
security.has_privileges:
user: null
body: >
{
"application": [
{
"application" : "myapp",
"resources" : [ "*" ],
"privileges" : [ "action:login" ]
},
{
"application" : "yourapp-v1",
"resources" : [ "*" ],
"privileges" : [ "read" ]
},
{
"application" : "yourapp-v2",
"resources" : [ "*" ],
"privileges" : [ "read" ]
}
]
}
- match: { "username" : "myapp_yourapp_wildard_role_user" }
- match: { "has_all_requested" : true }
- match: { "application" : {
"myapp" : {
"*" : {
"action:login" : true
}
},
"yourapp-v1": {
"*": {
"read": true
}
},
"yourapp-v2": {
"*": {
"read": true
}
}
} }