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

Short Circuit Filter Expression Security If User Checks would pass/fail. #1275

Merged
merged 9 commits into from
Apr 21, 2020
Prev Previous commit
Next Next commit
Added unit tests for changes to permission executor
Aaron Klish committed Apr 21, 2020

Unverified

The email in this signature doesn’t match the committer email.
commit 99b0de064e053afa659a747015f2aeeb2c05a448
Original file line number Diff line number Diff line change
@@ -91,12 +91,16 @@ private ExpressionResult evaluateUserChecks(Path path) {
ExpressionResult result = ExpressionResult.PASS;

for (Path.PathElement element : path.getPathElements()) {
result = executor.checkUserPermissions(
element.getType(),
ReadPermission.class,
element.getFieldName());
try {
result = executor.checkUserPermissions(
element.getType(),
ReadPermission.class,
element.getFieldName());
} catch (ForbiddenAccessException e) {
return ExpressionResult.FAIL;
}

if (result == ExpressionResult.DEFERRED || result == ExpressionResult.FAIL) {
if (result == ExpressionResult.DEFERRED) {
return result;
}
}
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@
*/
package com.yahoo.elide.security;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

import com.yahoo.elide.ElideSettings;
@@ -21,6 +22,7 @@
import com.yahoo.elide.security.checks.OperationCheck;
import com.yahoo.elide.security.checks.UserCheck;

import com.yahoo.elide.security.permissions.ExpressionResult;
import example.TestCheckMappings;

import org.junit.jupiter.api.Test;
@@ -413,6 +415,40 @@ public void testUserCheckCache() {
requestScope.getPermissionExecutor().checkPermission(ReadPermission.class, resource, cspec);
}

@Test
public void testUserCheckOnFieldSuccess() {
PersistentResource resource = newResource(OpenBean.class);
RequestScope requestScope = resource.getRequestScope();
ExpressionResult result = requestScope.getPermissionExecutor().checkUserPermissions(OpenBean.class,
ReadPermission.class,
"open");

assertEquals(ExpressionResult.PASS, result);
}

@Test
public void testUserCheckOnFieldFailure() {
PersistentResource resource = newResource(SampleBean.class);
RequestScope requestScope = resource.getRequestScope();
assertThrows(
ForbiddenAccessException.class,
() -> requestScope.getPermissionExecutor().checkUserPermissions(SampleBean.class,
ReadPermission.class,
"cannotSeeMe"));
}

@Test
public void testUserCheckOnFieldDeferred() {
PersistentResource resource = newResource(SampleBean.class);
RequestScope requestScope = resource.getRequestScope();

ExpressionResult result = requestScope.getPermissionExecutor().checkUserPermissions(SampleBean.class,
ReadPermission.class,
"allVisible");

assertEquals(ExpressionResult.DEFERRED, result);
}

public <T> PersistentResource<T> newResource(T obj, Class<T> cls) {
EntityDictionary dictionary = new EntityDictionary(TestCheckMappings.MAPPINGS);
dictionary.bindEntity(cls);