Repository: nifi Updated Branches: refs/heads/master 41f325344 -> f43f47694
NIFI-2138 Making AccessPolicy have a single RequestAction. This closes #590 Project: http://git-wip-us.apache.org/repos/asf/nifi/repo Commit: http://git-wip-us.apache.org/repos/asf/nifi/commit/f43f4769 Tree: http://git-wip-us.apache.org/repos/asf/nifi/tree/f43f4769 Diff: http://git-wip-us.apache.org/repos/asf/nifi/diff/f43f4769 Branch: refs/heads/master Commit: f43f47694c9d55617a068bed62f7d70267c25bee Parents: 41f3253 Author: Bryan Bende <bbe...@apache.org> Authored: Tue Jun 28 14:26:00 2016 -0400 Committer: Matt Gilman <matt.c.gil...@gmail.com> Committed: Tue Jun 28 16:32:27 2016 -0400 ---------------------------------------------------------------------- .../AbstractPolicyBasedAuthorizer.java | 36 +++---------- .../apache/nifi/authorization/AccessPolicy.java | 50 +++++------------ .../TestAbstractPolicyBasedAuthorizer.java | 16 +++--- .../nifi/authorization/TestAccessPolicy.java | 57 +++++--------------- .../authorization/AuthorizationsHolder.java | 11 ++-- .../nifi/authorization/FileAuthorizer.java | 32 +++++------ .../nifi/authorization/RoleAccessPolicy.java | 14 ++--- .../src/main/xsd/authorizations.xsd | 2 +- .../nifi/authorization/FileAuthorizerTest.java | 54 +++++++++---------- .../nifi/controller/TestFlowController.java | 5 +- .../org/apache/nifi/web/api/dto/DtoFactory.java | 8 +-- .../impl/StandardPolicyBasedAuthorizerDAO.java | 7 ++- .../StandardPolicyBasedAuthorizerDAOSpec.groovy | 10 ++-- 13 files changed, 110 insertions(+), 192 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/nifi/blob/f43f4769/nifi-api/src/main/java/org/apache/nifi/authorization/AbstractPolicyBasedAuthorizer.java ---------------------------------------------------------------------- diff --git a/nifi-api/src/main/java/org/apache/nifi/authorization/AbstractPolicyBasedAuthorizer.java b/nifi-api/src/main/java/org/apache/nifi/authorization/AbstractPolicyBasedAuthorizer.java index 71168f6..6a8467d 100644 --- a/nifi-api/src/main/java/org/apache/nifi/authorization/AbstractPolicyBasedAuthorizer.java +++ b/nifi-api/src/main/java/org/apache/nifi/authorization/AbstractPolicyBasedAuthorizer.java @@ -78,9 +78,8 @@ public abstract class AbstractPolicyBasedAuthorizer implements Authorizer { } for (AccessPolicy policy : policies) { - final boolean containsAction = policy.getActions().contains(request.getAction()); final boolean containsUser = policy.getUsers().contains(user.getIdentifier()); - if (containsAction && (containsUser || containsGroup(user, policy)) ) { + if (policy.getAction() == request.getAction() && (containsUser || containsGroup(user, policy)) ) { return AuthorizationResult.approved(); } } @@ -327,11 +326,12 @@ public abstract class AbstractPolicyBasedAuthorizer implements Authorizer { .resource(element.getAttribute(RESOURCE_ATTR)); final String actions = element.getAttribute(ACTIONS_ATTR); - if (actions.contains(RequestAction.READ.name())) { - builder.addAction(RequestAction.READ); - } - if (actions.contains(RequestAction.WRITE.name())) { - builder.addAction(RequestAction.WRITE); + if (actions.equals(RequestAction.READ.name())) { + builder.action(RequestAction.READ); + } else if (actions.equals(RequestAction.WRITE.name())) { + builder.action(RequestAction.WRITE); + } else { + throw new IllegalStateException("Unknown Policy Action: " + actions); } NodeList policyUsers = element.getElementsByTagName(POLICY_USER_ELEMENT); @@ -427,13 +427,6 @@ public abstract class AbstractPolicyBasedAuthorizer implements Authorizer { } private void writePolicy(final XMLStreamWriter writer, final AccessPolicy policy) throws XMLStreamException { - // build the action string in a deterministic order - StringBuilder actionBuilder = new StringBuilder(); - List<RequestAction> actions = getSortedActions(policy); - for (RequestAction action : actions) { - actionBuilder.append(action); - } - // sort the users for the policy List<String> policyUsers = new ArrayList<>(policy.getUsers()); Collections.sort(policyUsers); @@ -445,7 +438,7 @@ public abstract class AbstractPolicyBasedAuthorizer implements Authorizer { writer.writeStartElement(POLICY_ELEMENT); writer.writeAttribute(IDENTIFIER_ATTR, policy.getIdentifier()); writer.writeAttribute(RESOURCE_ATTR, policy.getResource()); - writer.writeAttribute(ACTIONS_ATTR, actionBuilder.toString()); + writer.writeAttribute(ACTIONS_ATTR, policy.getAction().name()); for (String policyUser : policyUsers) { writer.writeStartElement(POLICY_USER_ELEMENT); @@ -498,17 +491,4 @@ public abstract class AbstractPolicyBasedAuthorizer implements Authorizer { return users; } - private List<RequestAction> getSortedActions(final AccessPolicy policy) { - final List<RequestAction> actions = new ArrayList<>(policy.getActions()); - - Collections.sort(actions, new Comparator<RequestAction>() { - @Override - public int compare(RequestAction r1, RequestAction r2) { - return r1.name().compareTo(r2.name()); - } - }); - - return actions; - } - } http://git-wip-us.apache.org/repos/asf/nifi/blob/f43f4769/nifi-api/src/main/java/org/apache/nifi/authorization/AccessPolicy.java ---------------------------------------------------------------------- diff --git a/nifi-api/src/main/java/org/apache/nifi/authorization/AccessPolicy.java b/nifi-api/src/main/java/org/apache/nifi/authorization/AccessPolicy.java index 7ef40a3..93cabb2 100644 --- a/nifi-api/src/main/java/org/apache/nifi/authorization/AccessPolicy.java +++ b/nifi-api/src/main/java/org/apache/nifi/authorization/AccessPolicy.java @@ -34,14 +34,14 @@ public class AccessPolicy { private final Set<String> groups; - private final Set<RequestAction> actions; + private final RequestAction action; private AccessPolicy(final Builder builder) { this.identifier = builder.identifier; this.resource = builder.resource; + this.action = builder.action; this.users = Collections.unmodifiableSet(new HashSet<>(builder.users)); this.groups = Collections.unmodifiableSet(new HashSet<>(builder.groups)); - this.actions = Collections.unmodifiableSet(new HashSet<>(builder.actions)); if (this.identifier == null || this.identifier.trim().isEmpty()) { throw new IllegalArgumentException("Identifier can not be null or empty"); @@ -51,8 +51,8 @@ public class AccessPolicy { throw new IllegalArgumentException("Resource can not be null"); } - if (this.actions == null || this.actions.isEmpty()) { - throw new IllegalArgumentException("Actions can not be null or empty"); + if (this.action == null) { + throw new IllegalArgumentException("Action can not be null"); } } @@ -85,10 +85,10 @@ public class AccessPolicy { } /** - * @return an unmodifiable set of actions for this policy + * @return the action for this policy */ - public Set<RequestAction> getActions() { - return actions; + public RequestAction getAction() { + return action; } @Override @@ -112,7 +112,7 @@ public class AccessPolicy { @Override public String toString() { return String.format("identifier[%s], resource[%s], users[%s], groups[%s], action[%s]", - getIdentifier(), getResource(), getUsers(), getGroups(), getActions()); + getIdentifier(), getResource(), getUsers(), getGroups(), getAction()); } /** @@ -122,9 +122,9 @@ public class AccessPolicy { private String identifier; private String resource; + private RequestAction action; private Set<String> users = new HashSet<>(); private Set<String> groups = new HashSet<>(); - private Set<RequestAction> actions = new HashSet<>(); private final boolean fromPolicy; /** @@ -148,12 +148,11 @@ public class AccessPolicy { this.identifier = other.getIdentifier(); this.resource = other.getResource(); + this.action = other.getAction(); this.users.clear(); this.users.addAll(other.getUsers()); this.groups.clear(); this.groups.addAll(other.getGroups()); - this.actions.clear(); - this.actions.addAll(other.getActions()); this.fromPolicy = true; } @@ -310,34 +309,13 @@ public class AccessPolicy { } /** - * Adds the provided action to the builder's set of actions. + * Sets the action for this builder. * - * @param action the action to add + * @param action the action to set * @return the builder */ - public Builder addAction(final RequestAction action) { - this.actions.add(action); - return this; - } - - /** - * Removes the provided action from the builder's set of actions. - * - * @param action the action to remove - * @return the builder - */ - public Builder removeAction(final RequestAction action) { - this.actions.remove(action); - return this; - } - - /** - * Clears the builder's set of actions so that it is non-null and size == 0. - * - * @return the builder - */ - public Builder clearActions() { - this.actions.clear(); + public Builder action(final RequestAction action) { + this.action = action; return this; } http://git-wip-us.apache.org/repos/asf/nifi/blob/f43f4769/nifi-api/src/test/java/org/apache/nifi/authorization/TestAbstractPolicyBasedAuthorizer.java ---------------------------------------------------------------------- diff --git a/nifi-api/src/test/java/org/apache/nifi/authorization/TestAbstractPolicyBasedAuthorizer.java b/nifi-api/src/test/java/org/apache/nifi/authorization/TestAbstractPolicyBasedAuthorizer.java index 6215fac..50f27f3 100644 --- a/nifi-api/src/test/java/org/apache/nifi/authorization/TestAbstractPolicyBasedAuthorizer.java +++ b/nifi-api/src/test/java/org/apache/nifi/authorization/TestAbstractPolicyBasedAuthorizer.java @@ -58,7 +58,7 @@ public class TestAbstractPolicyBasedAuthorizer { .identifier("1") .resource(TEST_RESOURCE.getIdentifier()) .addUser(userIdentifier) - .addAction(RequestAction.READ) + .action(RequestAction.READ) .build()); when(usersAndAccessPolicies.getAccessPolicies(TEST_RESOURCE.getIdentifier())).thenReturn(policiesForResource); @@ -96,7 +96,7 @@ public class TestAbstractPolicyBasedAuthorizer { .identifier("1") .resource(TEST_RESOURCE.getIdentifier()) .addGroup(groupIdentifier) - .addAction(RequestAction.READ) + .action(RequestAction.READ) .build()); when(usersAndAccessPolicies.getAccessPolicies(TEST_RESOURCE.getIdentifier())).thenReturn(policiesForResource); @@ -134,7 +134,7 @@ public class TestAbstractPolicyBasedAuthorizer { .identifier("1") .resource(TEST_RESOURCE.getIdentifier()) .addUser("NOT_USER_1") - .addAction(RequestAction.READ) + .action(RequestAction.READ) .build()); when(usersAndAccessPolicies.getAccessPolicies(TEST_RESOURCE.getIdentifier())).thenReturn(policiesForResource); @@ -189,7 +189,7 @@ public class TestAbstractPolicyBasedAuthorizer { AccessPolicy policy1 = new AccessPolicy.Builder() .identifier("policy-id-1") .resource("resource1") - .addAction(RequestAction.READ) + .action(RequestAction.READ) .addUser(user1.getIdentifier()) .addUser(user2.getIdentifier()) .build(); @@ -197,8 +197,7 @@ public class TestAbstractPolicyBasedAuthorizer { AccessPolicy policy2 = new AccessPolicy.Builder() .identifier("policy-id-2") .resource("resource2") - .addAction(RequestAction.READ) - .addAction(RequestAction.WRITE) + .action(RequestAction.READ) .addGroup(group1.getIdentifier()) .addGroup(group2.getIdentifier()) .addUser(user1.getIdentifier()) @@ -261,7 +260,7 @@ public class TestAbstractPolicyBasedAuthorizer { AccessPolicy policy1 = new AccessPolicy.Builder() .identifier("policy-id-1") .resource("resource1") - .addAction(RequestAction.READ) + .action(RequestAction.READ) .addUser(user1.getIdentifier()) .addUser(user2.getIdentifier()) .build(); @@ -269,8 +268,7 @@ public class TestAbstractPolicyBasedAuthorizer { AccessPolicy policy2 = new AccessPolicy.Builder() .identifier("policy-id-2") .resource("resource2") - .addAction(RequestAction.READ) - .addAction(RequestAction.WRITE) + .action(RequestAction.READ) .addGroup(group1.getIdentifier()) .addGroup(group2.getIdentifier()) .addUser(user1.getIdentifier()) http://git-wip-us.apache.org/repos/asf/nifi/blob/f43f4769/nifi-api/src/test/java/org/apache/nifi/authorization/TestAccessPolicy.java ---------------------------------------------------------------------- diff --git a/nifi-api/src/test/java/org/apache/nifi/authorization/TestAccessPolicy.java b/nifi-api/src/test/java/org/apache/nifi/authorization/TestAccessPolicy.java index 35e1616..c8a4452 100644 --- a/nifi-api/src/test/java/org/apache/nifi/authorization/TestAccessPolicy.java +++ b/nifi-api/src/test/java/org/apache/nifi/authorization/TestAccessPolicy.java @@ -24,7 +24,6 @@ import java.util.Set; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; public class TestAccessPolicy { @@ -42,7 +41,7 @@ public class TestAccessPolicy { .resource(TEST_RESOURCE) .addUser(user1) .addUser(user2) - .addAction(action) + .action(action) .build(); assertEquals(identifier, policy.getIdentifier()); @@ -55,9 +54,8 @@ public class TestAccessPolicy { assertTrue(policy.getUsers().contains(user1)); assertTrue(policy.getUsers().contains(user2)); - assertNotNull(policy.getActions()); - assertEquals(1, policy.getActions().size()); - assertTrue(policy.getActions().contains(action)); + assertNotNull(policy.getAction()); + assertEquals(RequestAction.READ, policy.getAction()); } @Test(expected = IllegalArgumentException.class) @@ -65,7 +63,7 @@ public class TestAccessPolicy { new AccessPolicy.Builder() .resource(TEST_RESOURCE) .addUser("user1") - .addAction(RequestAction.READ) + .action(RequestAction.READ) .build(); } @@ -74,7 +72,7 @@ public class TestAccessPolicy { new AccessPolicy.Builder() .identifier("1") .addUser("user1") - .addAction(RequestAction.READ) + .action(RequestAction.READ) .build(); } @@ -83,7 +81,7 @@ public class TestAccessPolicy { final AccessPolicy policy = new AccessPolicy.Builder() .identifier("1") .resource(TEST_RESOURCE) - .addAction(RequestAction.READ) + .action(RequestAction.READ) .build(); assertNotNull(policy); @@ -114,7 +112,7 @@ public class TestAccessPolicy { .addUser(user2) .addGroup(group1) .addGroup(group2) - .addAction(action) + .action(action) .build(); assertEquals(identifier, policy.getIdentifier()); @@ -132,15 +130,14 @@ public class TestAccessPolicy { assertTrue(policy.getGroups().contains(group1)); assertTrue(policy.getGroups().contains(group2)); - assertNotNull(policy.getActions()); - assertEquals(1, policy.getActions().size()); - assertTrue(policy.getActions().contains(action)); + assertNotNull(policy.getAction()); + assertEquals(RequestAction.READ, policy.getAction()); final AccessPolicy policy2 = new AccessPolicy.Builder(policy).build(); assertEquals(policy.getIdentifier(), policy2.getIdentifier()); assertEquals(policy.getResource(), policy2.getResource()); assertEquals(policy.getUsers(), policy2.getUsers()); - assertEquals(policy.getActions(), policy2.getActions()); + assertEquals(policy.getAction(), policy2.getAction()); } @Test(expected = IllegalStateException.class) @@ -149,7 +146,7 @@ public class TestAccessPolicy { .identifier("1") .resource(TEST_RESOURCE) .addUser("user1") - .addAction(RequestAction.READ) + .action(RequestAction.READ) .build(); new AccessPolicy.Builder(policy).identifier("2").build(); @@ -161,7 +158,7 @@ public class TestAccessPolicy { .identifier("1") .resource(TEST_RESOURCE) .addUser("user1") - .addAction(RequestAction.READ); + .action(RequestAction.READ); final AccessPolicy policy1 = builder.build(); assertEquals(1, policy1.getUsers().size()); @@ -204,7 +201,7 @@ public class TestAccessPolicy { .identifier("1") .resource(TEST_RESOURCE) .addGroup("group1") - .addAction(RequestAction.READ); + .action(RequestAction.READ); final AccessPolicy policy1 = builder.build(); assertEquals(1, policy1.getGroups().size()); @@ -240,32 +237,4 @@ public class TestAccessPolicy { assertEquals(0, policy5.getUsers().size()); } - @Test - public void testAddRemoveClearActions() { - final AccessPolicy.Builder builder = new AccessPolicy.Builder() - .identifier("1") - .resource(TEST_RESOURCE) - .addUser("user1") - .addAction(RequestAction.READ); - - final AccessPolicy policy1 = builder.build(); - assertEquals(1, policy1.getActions().size()); - assertTrue(policy1.getActions().contains(RequestAction.READ)); - - final AccessPolicy policy2 = builder - .removeAction(RequestAction.READ) - .addAction(RequestAction.WRITE) - .build(); - - assertEquals(1, policy2.getActions().size()); - assertTrue(policy2.getActions().contains(RequestAction.WRITE)); - - try { - builder.clearActions().build(); - fail("should have thrown exception"); - } catch (IllegalArgumentException e) { - - } - } - } http://git-wip-us.apache.org/repos/asf/nifi/blob/f43f4769/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizationsHolder.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizationsHolder.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizationsHolder.java index e98eed9..62ab2d3 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizationsHolder.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizationsHolder.java @@ -126,11 +126,12 @@ public class AuthorizationsHolder implements UsersAndAccessPolicies { // add the appropriate request actions final String authorizationCode = policy.getAction(); - if (authorizationCode.contains(FileAuthorizer.READ_CODE)) { - builder.addAction(RequestAction.READ); - } - if (authorizationCode.contains(FileAuthorizer.WRITE_CODE)) { - builder.addAction(RequestAction.WRITE); + if (authorizationCode.equals(FileAuthorizer.READ_CODE)) { + builder.action(RequestAction.READ); + } else if (authorizationCode.equals(FileAuthorizer.WRITE_CODE)){ + builder.action(RequestAction.WRITE); + } else { + throw new IllegalStateException("Unknown Policy Action: " + authorizationCode); } // build the policy and add it to the map http://git-wip-us.apache.org/repos/asf/nifi/blob/f43f4769/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileAuthorizer.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileAuthorizer.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileAuthorizer.java index 5c8c9ff..7c04298 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileAuthorizer.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileAuthorizer.java @@ -103,7 +103,6 @@ public class FileAuthorizer extends AbstractPolicyBasedAuthorizer { static final String PROP_AUTHORIZATIONS_FILE = "Authorizations File"; static final String PROP_INITIAL_ADMIN_IDENTITY = "Initial Admin Identity"; static final String PROP_LEGACY_AUTHORIZED_USERS_FILE = "Legacy Authorized Users File"; - static final String PROP_ROOT_GROUP_ID = "Root Group ID"; private Schema flowSchema; private Schema usersSchema; @@ -325,14 +324,14 @@ public class FileAuthorizer extends AbstractPolicyBasedAuthorizer { // grant the user read access to the root process group resource if (rootGroupId != null) { - addAccessPolicy(authorizations, ResourceType.ProcessGroup.getValue() + "/" + rootGroupId, adminUser.getIdentifier(), READ_CODE + WRITE_CODE); + addAccessPolicy(authorizations, ResourceType.ProcessGroup.getValue() + "/" + rootGroupId, adminUser.getIdentifier(), WRITE_CODE); } // grant the user read/write access to the /tenants resource - addAccessPolicy(authorizations, ResourceType.Tenant.getValue(), adminUser.getIdentifier(), READ_CODE + WRITE_CODE); + addAccessPolicy(authorizations, ResourceType.Tenant.getValue(), adminUser.getIdentifier(), WRITE_CODE); // grant the user read/write access to the /policies resource - addAccessPolicy(authorizations, ResourceType.Policy.getValue(), adminUser.getIdentifier(), READ_CODE + WRITE_CODE); + addAccessPolicy(authorizations, ResourceType.Policy.getValue(), adminUser.getIdentifier(), WRITE_CODE); } /** @@ -578,9 +577,9 @@ public class FileAuthorizer extends AbstractPolicyBasedAuthorizer { * @param authorizations the Authorizations instance to add the policy to * @param resource the resource for the policy * @param identity the identity for the policy - * @param actions the actions for the policy + * @param action the action for the policy */ - private void addAccessPolicy(final Authorizations authorizations, final String resource, final String identity, final String actions) { + private void addAccessPolicy(final Authorizations authorizations, final String resource, final String identity, final String action) { final String uuidSeed = resource + identity; final UUID policyIdentifier = UUID.nameUUIDFromBytes(uuidSeed.getBytes(StandardCharsets.UTF_8)); @@ -589,12 +588,12 @@ public class FileAuthorizer extends AbstractPolicyBasedAuthorizer { .resource(resource) .addUser(identity); - if (actions.contains(READ_CODE)) { - builder.addAction(RequestAction.READ); - } - - if (actions.contains(WRITE_CODE)) { - builder.addAction(RequestAction.WRITE); + if (action.equals(READ_CODE)) { + builder.action(RequestAction.READ); + } else if (action.equals(WRITE_CODE)) { + builder.action(RequestAction.WRITE); + } else { + throw new IllegalStateException("Unknown Policy Action: " + action); } final AccessPolicy accessPolicy = builder.build(); @@ -1093,13 +1092,8 @@ public class FileAuthorizer extends AbstractPolicyBasedAuthorizer { policy.getGroup().add(policyGroup); } - // add the action to the policy - boolean containsRead = accessPolicy.getActions().contains(RequestAction.READ); - boolean containsWrite = accessPolicy.getActions().contains(RequestAction.WRITE); - - if (containsRead && containsWrite) { - policy.setAction(READ_CODE + WRITE_CODE); - } else if (containsRead) { + // add the action to the access policy + if (accessPolicy.getAction() == RequestAction.READ) { policy.setAction(READ_CODE); } else { policy.setAction(WRITE_CODE); http://git-wip-us.apache.org/repos/asf/nifi/blob/f43f4769/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/RoleAccessPolicy.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/RoleAccessPolicy.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/RoleAccessPolicy.java index 32b7ce9..24857f1 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/RoleAccessPolicy.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/RoleAccessPolicy.java @@ -30,7 +30,7 @@ import java.util.Set; public final class RoleAccessPolicy { static final String READ_ACTION = "R"; - static final String READ_WRITE_ACTION = "RW"; + static final String WRITE_ACTION = "W"; private final String resource; private final String actions; @@ -66,10 +66,10 @@ public final class RoleAccessPolicy { final Set<RoleAccessPolicy> dfmPolicies = new HashSet<>(); dfmPolicies.add(new RoleAccessPolicy(ResourceType.Flow.getValue(), READ_ACTION)); - dfmPolicies.add(new RoleAccessPolicy(ResourceType.Controller.getValue(), READ_WRITE_ACTION)); + dfmPolicies.add(new RoleAccessPolicy(ResourceType.Controller.getValue(), WRITE_ACTION)); dfmPolicies.add(new RoleAccessPolicy(ResourceType.System.getValue(), READ_ACTION)); if (rootGroupId != null) { - dfmPolicies.add(new RoleAccessPolicy(ResourceType.ProcessGroup.getValue() + "/" + rootGroupId, READ_WRITE_ACTION)); + dfmPolicies.add(new RoleAccessPolicy(ResourceType.ProcessGroup.getValue() + "/" + rootGroupId, WRITE_ACTION)); } roleAccessPolicies.put(Role.ROLE_DFM, Collections.unmodifiableSet(dfmPolicies)); @@ -79,17 +79,17 @@ public final class RoleAccessPolicy { if (rootGroupId != null) { adminPolicies.add(new RoleAccessPolicy(ResourceType.ProcessGroup.getValue() + "/" + rootGroupId, READ_ACTION)); } - adminPolicies.add(new RoleAccessPolicy(ResourceType.Tenant.getValue(), READ_WRITE_ACTION)); - adminPolicies.add(new RoleAccessPolicy(ResourceType.Policy.getValue(), READ_WRITE_ACTION)); + adminPolicies.add(new RoleAccessPolicy(ResourceType.Tenant.getValue(), WRITE_ACTION)); + adminPolicies.add(new RoleAccessPolicy(ResourceType.Policy.getValue(), WRITE_ACTION)); roleAccessPolicies.put(Role.ROLE_ADMIN, Collections.unmodifiableSet(adminPolicies)); final Set<RoleAccessPolicy> proxyPolicies = new HashSet<>(); - proxyPolicies.add(new RoleAccessPolicy(ResourceType.Proxy.getValue(), READ_WRITE_ACTION)); + proxyPolicies.add(new RoleAccessPolicy(ResourceType.Proxy.getValue(), WRITE_ACTION)); roleAccessPolicies.put(Role.ROLE_PROXY, Collections.unmodifiableSet(proxyPolicies)); final Set<RoleAccessPolicy> nifiPolicies = new HashSet<>(); nifiPolicies.add(new RoleAccessPolicy(ResourceType.Controller.getValue(), READ_ACTION)); - nifiPolicies.add(new RoleAccessPolicy(ResourceType.SiteToSite.getValue(), READ_WRITE_ACTION)); + nifiPolicies.add(new RoleAccessPolicy(ResourceType.SiteToSite.getValue(), WRITE_ACTION)); roleAccessPolicies.put(Role.ROLE_NIFI, Collections.unmodifiableSet(nifiPolicies)); return roleAccessPolicies; http://git-wip-us.apache.org/repos/asf/nifi/blob/f43f4769/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/xsd/authorizations.xsd ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/xsd/authorizations.xsd b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/xsd/authorizations.xsd index f3e220d..9b24c7a 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/xsd/authorizations.xsd +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/xsd/authorizations.xsd @@ -131,7 +131,7 @@ <xs:simpleType> <xs:restriction base="xs:string"> <xs:enumeration value="R"/> - <xs:enumeration value="RW"/> + <xs:enumeration value="W"/> </xs:restriction> </xs:simpleType> </xs:attribute> http://git-wip-us.apache.org/repos/asf/nifi/blob/f43f4769/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileAuthorizerTest.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileAuthorizerTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileAuthorizerTest.java index 8d49928..fc08e71 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileAuthorizerTest.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileAuthorizerTest.java @@ -90,12 +90,12 @@ public class FileAuthorizerTest { " <user identifier=\"user-2\" identity=\"user-2\" />" + " </users>" + " <policies>" + - " <policy identifier=\"policy-1\" resource=\"/flow\" action=\"RW\">" + + " <policy identifier=\"policy-1\" resource=\"/flow\" action=\"R\">" + " <group identifier=\"group-1\" />" + " <group identifier=\"group-2\" />" + " <user identifier=\"user-1\" />" + " </policy>" + - " <policy identifier=\"policy-2\" resource=\"/flow\" action=\"RW\">" + + " <policy identifier=\"policy-2\" resource=\"/flow\" action=\"W\">" + " <user identifier=\"user-2\" />" + " </policy>" + " </policies>" + @@ -231,7 +231,8 @@ public class FileAuthorizerTest { assertTrue(user3Policies.get(ResourceType.Flow.getValue()).contains(RequestAction.READ)); assertTrue(user3Policies.containsKey(ResourceType.ProcessGroup.getValue() + "/" + ROOT_GROUP_ID)); - assertEquals(2, user3Policies.get(ResourceType.ProcessGroup.getValue() + "/" + ROOT_GROUP_ID).size()); + assertEquals(1, user3Policies.get(ResourceType.ProcessGroup.getValue() + "/" + ROOT_GROUP_ID).size()); + assertTrue(user3Policies.get(ResourceType.ProcessGroup.getValue() + "/" + ROOT_GROUP_ID).contains(RequestAction.WRITE)); // verify user4's policies final Map<String,Set<RequestAction>> user4Policies = getResourceActions(policies, user4); @@ -246,24 +247,28 @@ public class FileAuthorizerTest { assertTrue(user4Policies.get(ResourceType.ProcessGroup.getValue() + "/" + ROOT_GROUP_ID).contains(RequestAction.READ)); assertTrue(user4Policies.containsKey(ResourceType.Tenant.getValue())); - assertEquals(2, user4Policies.get(ResourceType.Tenant.getValue()).size()); + assertEquals(1, user4Policies.get(ResourceType.Tenant.getValue()).size()); + assertTrue(user4Policies.get(ResourceType.Tenant.getValue()).contains(RequestAction.WRITE)); assertTrue(user4Policies.containsKey(ResourceType.Policy.getValue())); - assertEquals(2, user4Policies.get(ResourceType.Policy.getValue()).size()); + assertEquals(1, user4Policies.get(ResourceType.Policy.getValue()).size()); + assertTrue(user4Policies.get(ResourceType.Policy.getValue()).contains(RequestAction.WRITE)); // verify user5's policies final Map<String,Set<RequestAction>> user5Policies = getResourceActions(policies, user5); assertEquals(1, user5Policies.size()); assertTrue(user5Policies.containsKey(ResourceType.Proxy.getValue())); - assertEquals(2, user5Policies.get(ResourceType.Proxy.getValue()).size()); + assertEquals(1, user5Policies.get(ResourceType.Proxy.getValue()).size()); + assertTrue(user5Policies.get(ResourceType.Proxy.getValue()).contains(RequestAction.WRITE)); // verify user6's policies final Map<String,Set<RequestAction>> user6Policies = getResourceActions(policies, user6); assertEquals(2, user6Policies.size()); assertTrue(user6Policies.containsKey(ResourceType.SiteToSite.getValue())); - assertEquals(2, user6Policies.get(ResourceType.SiteToSite.getValue()).size()); + assertEquals(1, user6Policies.get(ResourceType.SiteToSite.getValue()).size()); + assertTrue(user6Policies.get(ResourceType.SiteToSite.getValue()).contains(RequestAction.WRITE)); } private Map<String,Set<RequestAction>> getResourceActions(final Set<AccessPolicy> policies, final User user) { @@ -276,7 +281,7 @@ public class FileAuthorizerTest { actions = new HashSet<>(); resourceActionMap.put(accessPolicy.getResource(), actions); } - actions.addAll(accessPolicy.getActions()); + actions.add(accessPolicy.getAction()); } } @@ -561,9 +566,7 @@ public class FileAuthorizerTest { for (AccessPolicy policy : policies) { if (policy.getIdentifier().equals("policy-1") && policy.getResource().equals("/flow") - && policy.getActions().size() == 2 - && policy.getActions().contains(RequestAction.READ) - && policy.getActions().contains(RequestAction.WRITE) + && policy.getAction() == RequestAction.READ && policy.getGroups().size() == 2 && policy.getGroups().contains("group-1") && policy.getGroups().contains("group-2") @@ -572,9 +575,7 @@ public class FileAuthorizerTest { foundPolicy1 = true; } else if (policy.getIdentifier().equals("policy-2") && policy.getResource().equals("/flow") - && policy.getActions().size() == 2 - && policy.getActions().contains(RequestAction.READ) - && policy.getActions().contains(RequestAction.WRITE) + && policy.getAction() == RequestAction.WRITE && policy.getGroups().size() == 0 && policy.getUsers().size() == 1 && policy.getUsers().contains("user-2")) { @@ -944,7 +945,7 @@ public class FileAuthorizerTest { .resource("resource-1") .addUser("user-1") .addGroup("group-1") - .addAction(RequestAction.READ) + .action(RequestAction.READ) .build(); final AccessPolicy returnedPolicy1 = authorizer.addAccessPolicy(policy1); @@ -953,7 +954,7 @@ public class FileAuthorizerTest { assertEquals(policy1.getResource(), returnedPolicy1.getResource()); assertEquals(policy1.getUsers(), returnedPolicy1.getUsers()); assertEquals(policy1.getGroups(), returnedPolicy1.getGroups()); - assertEquals(policy1.getActions(), returnedPolicy1.getActions()); + assertEquals(policy1.getAction(), returnedPolicy1.getAction()); assertEquals(1, authorizer.getAccessPolicies().size()); @@ -963,7 +964,7 @@ public class FileAuthorizerTest { .resource("resource-1") .addUser("user-1") .addGroup("group-1") - .addAction(RequestAction.READ) + .action(RequestAction.READ) .build(); final AccessPolicy returnedPolicy2 = authorizer.addAccessPolicy(policy2); @@ -980,7 +981,7 @@ public class FileAuthorizerTest { final AccessPolicy policy1 = new AccessPolicy.Builder() .identifier("policy-1") .resource("resource-1") - .addAction(RequestAction.READ) + .action(RequestAction.READ) .build(); final AccessPolicy returnedPolicy1 = authorizer.addAccessPolicy(policy1); @@ -989,7 +990,7 @@ public class FileAuthorizerTest { assertEquals(policy1.getResource(), returnedPolicy1.getResource()); assertEquals(policy1.getUsers(), returnedPolicy1.getUsers()); assertEquals(policy1.getGroups(), returnedPolicy1.getGroups()); - assertEquals(policy1.getActions(), returnedPolicy1.getActions()); + assertEquals(policy1.getAction(), returnedPolicy1.getAction()); assertEquals(1, authorizer.getAccessPolicies().size()); } @@ -1005,9 +1006,7 @@ public class FileAuthorizerTest { assertEquals("policy-1", policy.getIdentifier()); assertEquals("/flow", policy.getResource()); - assertEquals(2, policy.getActions().size()); - assertTrue(policy.getActions().contains(RequestAction.WRITE)); - assertTrue(policy.getActions().contains(RequestAction.READ)); + assertEquals(RequestAction.READ, policy.getAction()); assertEquals(1, policy.getUsers().size()); assertTrue(policy.getUsers().contains("user-1")); @@ -1038,7 +1037,7 @@ public class FileAuthorizerTest { .resource("resource-A") .addUser("user-A") .addGroup("group-A") - .addAction(RequestAction.READ) + .action(RequestAction.READ) .build(); final AccessPolicy updateAccessPolicy = authorizer.updateAccessPolicy(policy); @@ -1052,8 +1051,7 @@ public class FileAuthorizerTest { assertEquals(1, updateAccessPolicy.getGroups().size()); assertTrue(updateAccessPolicy.getGroups().contains("group-A")); - assertEquals(1, updateAccessPolicy.getActions().size()); - assertTrue(updateAccessPolicy.getActions().contains(RequestAction.READ)); + assertEquals(RequestAction.READ, updateAccessPolicy.getAction()); } @Test @@ -1067,7 +1065,7 @@ public class FileAuthorizerTest { .resource("resource-A") .addUser("user-A") .addGroup("group-A") - .addAction(RequestAction.READ) + .action(RequestAction.READ) .build(); final AccessPolicy updateAccessPolicy = authorizer.updateAccessPolicy(policy); @@ -1085,7 +1083,7 @@ public class FileAuthorizerTest { .resource("resource-A") .addUser("user-A") .addGroup("group-A") - .addAction(RequestAction.READ) + .action(RequestAction.READ) .build(); final AccessPolicy deletedAccessPolicy = authorizer.deleteAccessPolicy(policy); @@ -1108,7 +1106,7 @@ public class FileAuthorizerTest { .resource("resource-A") .addUser("user-A") .addGroup("group-A") - .addAction(RequestAction.READ) + .action(RequestAction.READ) .build(); final AccessPolicy deletedAccessPolicy = authorizer.deleteAccessPolicy(policy); http://git-wip-us.apache.org/repos/asf/nifi/blob/f43f4769/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/TestFlowController.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/TestFlowController.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/TestFlowController.java index bddd741..ea12ad5 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/TestFlowController.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/TestFlowController.java @@ -80,7 +80,7 @@ public class TestFlowController { AccessPolicy policy1 = new AccessPolicy.Builder() .identifier("policy-id-1") .resource("resource1") - .addAction(RequestAction.READ) + .action(RequestAction.READ) .addUser(user1.getIdentifier()) .addUser(user2.getIdentifier()) .build(); @@ -88,8 +88,7 @@ public class TestFlowController { AccessPolicy policy2 = new AccessPolicy.Builder() .identifier("policy-id-2") .resource("resource2") - .addAction(RequestAction.READ) - .addAction(RequestAction.WRITE) + .action(RequestAction.READ) .addGroup(group1.getIdentifier()) .addGroup(group2.getIdentifier()) .addUser(user1.getIdentifier()) http://git-wip-us.apache.org/repos/asf/nifi/blob/f43f4769/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/dto/DtoFactory.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/dto/DtoFactory.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/dto/DtoFactory.java index 80ea9c9..8889292 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/dto/DtoFactory.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/dto/DtoFactory.java @@ -1562,9 +1562,11 @@ public final class DtoFactory { dto.setUsers(users); dto.setId(accessPolicy.getIdentifier()); dto.setResource(accessPolicy.getResource()); - Set<RequestAction> accessPolicyActions = accessPolicy.getActions(); - dto.setCanRead(accessPolicyActions.contains(RequestAction.READ)); - dto.setCanWrite(accessPolicyActions.contains(RequestAction.WRITE)); + if (accessPolicy.getAction() == RequestAction.WRITE) { + dto.setCanWrite(Boolean.TRUE); + } else { + dto.setCanRead(Boolean.TRUE); + } return dto; http://git-wip-us.apache.org/repos/asf/nifi/blob/f43f4769/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAO.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAO.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAO.java index 845d9f4..4aae278 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAO.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAO.java @@ -192,11 +192,10 @@ public class StandardPolicyBasedAuthorizerDAO implements AccessPolicyDAO, UserGr if (users != null) { builder.addUsers(users.stream().map(ComponentEntity::getId).collect(Collectors.toSet())); } - if (Boolean.TRUE == accessPolicyDTO.getCanRead()) { - builder.addAction(RequestAction.READ); - } if (Boolean.TRUE == accessPolicyDTO.getCanWrite()) { - builder.addAction(RequestAction.WRITE); + builder.action(RequestAction.WRITE); + } else { + builder.action(RequestAction.READ); } return builder.build(); } http://git-wip-us.apache.org/repos/asf/nifi/blob/f43f4769/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/groovy/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAOSpec.groovy ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/groovy/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAOSpec.groovy b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/groovy/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAOSpec.groovy index 78e9084..6c2686b 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/groovy/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAOSpec.groovy +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/groovy/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAOSpec.groovy @@ -79,7 +79,7 @@ class StandardPolicyBasedAuthorizerDAOSpec extends Specification { where: accessPolicy | _ new AccessPolicy.Builder().identifier('policy-id-1').resource('/fake/resource').addUser('user-id-1').addGroup('user-group-id-1') - .addAction(RequestAction.READ).addAction(RequestAction.WRITE).build() | _ + .action(RequestAction.WRITE).build() | _ null | _ } @@ -107,7 +107,7 @@ class StandardPolicyBasedAuthorizerDAOSpec extends Specification { where: accessPolicy | _ new AccessPolicy.Builder().identifier('policy-id-1').resource('/fake/resource').addUser('user-id-1').addGroup('user-group-id-1') - .addAction(RequestAction.READ).addAction(RequestAction.WRITE).build() | _ + .action(RequestAction.WRITE).build() | _ } @Unroll @@ -127,7 +127,7 @@ class StandardPolicyBasedAuthorizerDAOSpec extends Specification { where: accessPolicy | _ new AccessPolicy.Builder().identifier('policy-id-1').resource('/fake/resource').addUser('user-id-1').addGroup('user-group-id-1') - .addAction(RequestAction.READ).addAction(RequestAction.WRITE).build() | _ + .action(RequestAction.WRITE).build() | _ } @Unroll @@ -167,7 +167,7 @@ class StandardPolicyBasedAuthorizerDAOSpec extends Specification { where: accessPolicy | _ new AccessPolicy.Builder().identifier('policy-id-1').resource('/fake/resource').addUser('user-id-1').addGroup('user-group-id-1') - .addAction(RequestAction.READ).addAction(RequestAction.WRITE).build() | _ + .action(RequestAction.WRITE).build() | _ } @Unroll @@ -207,7 +207,7 @@ class StandardPolicyBasedAuthorizerDAOSpec extends Specification { where: accessPolicy | _ new AccessPolicy.Builder().identifier('policy-id-1').resource('/fake/resource').addUser('user-id-1').addGroup('user-group-id-1') - .addAction(RequestAction.READ).addAction(RequestAction.WRITE).build() | _ + .action(RequestAction.WRITE).build() | _ } @Unroll
