Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure that field-level security is enforced when using field aliases. #31807

Merged
merged 3 commits into from
Jul 16, 2018

Conversation

jtibshirani
Copy link
Contributor

@jtibshirani jtibshirani commented Jul 4, 2018

A user should only ever set permissions on a concrete field, not a field alias. Given this assumption, this PR verifies that access control is enforced in the following requests:

  • Search: security applies out-of-the-box because aliases are always translated to concrete fields before calling into lucene, and field level security is performed at the lucene level.
  • Field capabilities: when a field alias is used, we make sure to check the security filter against its target to avoid returning mapping information that a user doesn't have access to.

Note that even if a user can't see an alias' target, they are still able to retrieve the mappings for the alias (which includes the name of the target field). I looked into filtering alias mappings as well, but the change is quite involved. To me this seems like an okay trade-off for now -- when using a field alias, a user can tell that the target field exists, but cannot access any of its mapping information, whether through a 'get mappings' or field capabilities request.

I plan to update the documentation in elastic/stack-docs in a follow-up PR to specify that security shouldn't be set on field aliases. We currently don't enforce that permissions can't be set on an alias -- I didn't see a good way to do this, but am very happy for pointers.

@jtibshirani jtibshirani added >enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC labels Jul 4, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

Copy link
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

.setQuery(matchQuery("alias", "value1"))
.get();
assertHitCount(response, 1);
// user2 has no access to field1, so a query on its field alias should match with the document:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/should match/should not match

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jtibshirani jtibshirani merged commit e9b4008 into elastic:field-aliases Jul 16, 2018
@jtibshirani jtibshirani deleted the field-alias-security branch July 16, 2018 21:17
jtibshirani added a commit that referenced this pull request Jul 17, 2018
…ses. (#31807)

* Add basic unit tests for field level security with field aliases.
* Ensure that field caps information is filtered when a field alias is provided.
jtibshirani added a commit that referenced this pull request Jul 18, 2018
…ses. (#31807)

* Add basic unit tests for field level security with field aliases.
* Ensure that field caps information is filtered when a field alias is provided.
jtibshirani added a commit that referenced this pull request Jul 18, 2018
* Add basic support for field aliases in index mappings. (#31287)
* Allow for aliases when fetching stored fields. (#31411)
* Add tests around accessing field aliases in scripts. (#31417)
* Add documentation around field aliases. (#31538)
* Add validation for field alias mappings. (#31518)
* Return both concrete fields and aliases in DocumentFieldMappers#getMapper. (#31671)
* Make sure that field-level security is enforced when using field aliases. (#31807)
* Add more comprehensive tests for field aliases in queries + aggregations. (#31565)
* Remove the deprecated method DocumentFieldMappers#getFieldMapper. (#32148)
jtibshirani added a commit to jtibshirani/elasticsearch that referenced this pull request Jul 24, 2018
…ses. (elastic#31807)

* Add basic unit tests for field level security with field aliases.
* Ensure that field caps information is filtered when a field alias is provided.
jtibshirani added a commit that referenced this pull request Jul 24, 2018
* Add basic support for field aliases in index mappings. (#31287)
* Allow for aliases when fetching stored fields. (#31411)
* Add tests around accessing field aliases in scripts. (#31417)
* Return both concrete fields and aliases in DocumentFieldMappers#getMapper. (#31671)
* Add documentation around field aliases. (#31538)
* Add validation for field alias mappings. (#31518)
* Make sure that field-level security is enforced when using field aliases. (#31807)
* Add more comprehensive tests for field aliases in queries + aggregations. (#31565)
* Remove the deprecated method DocumentFieldMappers#getFieldMapper. (#32148)
* Ensure that field aliases cannot be used in multi-fields. (#32219)
* Make sure that field aliases count towards the total fields limit. (#32222)
* Fix a test bug around nested aggregations and field aliases. (#32287)
* Make sure the _uid field is correctly loaded in scripts.
* Fix the failing test case FieldLevelSecurityTests#testParentChild_parentField.
* Enforce that field aliases can only be specified on indexes with a single type.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants