Author: jawi
Date: Wed Nov 14 08:56:04 2012
New Revision: 1409119
URL: http://svn.apache.org/viewvc?rev=1409119&view=rev
Log:
FELIX-3755: removing roles did not remove them as group member;
- remove the role from all groups it is member of;
- some additional cleanups and unused code removed.
Modified:
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/AuthorizationImpl.java
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/RoleChecker.java
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/RoleRepository.java
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/role/GroupImpl.java
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/role/RoleImpl.java
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/role/UserImpl.java
felix/trunk/useradmin/useradmin/src/test/java/org/apache/felix/useradmin/impl/UserAdminImplTest.java
Modified:
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/AuthorizationImpl.java
URL:
http://svn.apache.org/viewvc/felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/AuthorizationImpl.java?rev=1409119&r1=1409118&r2=1409119&view=diff
==============================================================================
---
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/AuthorizationImpl.java
(original)
+++
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/AuthorizationImpl.java
Wed Nov 14 08:56:04 2012
@@ -21,8 +21,6 @@ import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
-
-import org.apache.felix.useradmin.impl.role.RoleImpl;
import org.osgi.service.useradmin.Authorization;
import org.osgi.service.useradmin.Role;
import org.osgi.service.useradmin.User;
@@ -86,7 +84,7 @@ public class AuthorizationImpl implement
Iterator rolesIter = m_roleManager.getRoles(null /* filter
*/).iterator();
while (rolesIter.hasNext()) {
- RoleImpl role = (RoleImpl) rolesIter.next();
+ Role role = (Role) rolesIter.next();
if (!Role.USER_ANYONE.equals(role.getName()) &&
m_roleChecker.isImpliedBy(role, m_user)) {
result.add(role.getName());
}
Modified:
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/RoleChecker.java
URL:
http://svn.apache.org/viewvc/felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/RoleChecker.java?rev=1409119&r1=1409118&r2=1409119&view=diff
==============================================================================
---
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/RoleChecker.java
(original)
+++
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/RoleChecker.java
Wed Nov 14 08:56:04 2012
@@ -19,11 +19,8 @@ package org.apache.felix.useradmin.impl;
import java.util.ArrayList;
import java.util.List;
-
-import org.apache.felix.useradmin.impl.role.UserImpl;
import org.osgi.service.useradmin.Group;
import org.osgi.service.useradmin.Role;
-import org.osgi.service.useradmin.User;
/**
* Helper class to check for implied role memberships.
@@ -40,9 +37,7 @@ final class RoleChecker {
public boolean isImpliedBy(Role role, Role impliedRole) {
if (role instanceof Group) {
return isGroupImpliedBy((Group) role, impliedRole, new
ArrayList());
- } else if (role instanceof User) {
- return isUserImpliedBy((User) role, impliedRole);
- } else {
+ } else /* if ((role instanceof User) || (role instanceof Role)) */ {
return isRoleImpliedBy(role, impliedRole);
}
}
@@ -72,10 +67,8 @@ final class RoleChecker {
if (requiredRole instanceof Group) {
seenGroups.add(requiredRole);
isImplied = isGroupImpliedBy((Group) requiredRole,
impliedRole, seenGroups);
- } else if (requiredRole instanceof User) {
- isImplied = isUserImpliedBy((User) requiredRole, impliedRole);
- } else /* if (requiredRoles[i] instanceof RoleImpl) */ {
- isImplied = isRoleImpliedBy(requiredRole, impliedRole);
+ } else /* if ((requiredRole instanceof User) || (requiredRole
instanceof Role)) */ {
+ isImplied = isRoleImpliedBy(requiredRole, impliedRole);
}
}
@@ -98,9 +91,7 @@ final class RoleChecker {
if (basicRole instanceof Group) {
seenGroups.add(basicRole);
isImplied = isGroupImpliedBy((Group) basicRole, impliedRole,
seenGroups);
- } else if (basicRole instanceof UserImpl) {
- isImplied = isUserImpliedBy((User) basicRole, impliedRole);
- } else /* if (requiredRoles[i] instanceof RoleImpl) */ {
+ } else /* if ((basicRole instanceof User) || (basicRole instanceof
Role)) */ {
isImplied = isRoleImpliedBy(basicRole, impliedRole);
}
}
@@ -109,17 +100,6 @@ final class RoleChecker {
}
/**
- * Verifies whether the given user is implied by the given role.
- *
- * @param user the user to check, cannot be <code>null</code>;
- * @param impliedRole the implied role to check for, cannot be
<code>null</code>;
- * @return <code>true</code> if the given user is implied by the given
role, <code>false</code> otherwise.
- */
- private boolean isUserImpliedBy(User user, Role impliedRole) {
- return Role.USER_ANYONE.equals(user.getName()) || (impliedRole != null
&& impliedRole.getName().equals(user.getName()));
- }
-
- /**
* Verifies whether the given role is implied by the given role.
*
* @param role the role to check, cannot be <code>null</code>;
Modified:
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/RoleRepository.java
URL:
http://svn.apache.org/viewvc/felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/RoleRepository.java?rev=1409119&r1=1409118&r2=1409119&view=diff
==============================================================================
---
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/RoleRepository.java
(original)
+++
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/RoleRepository.java
Wed Nov 14 08:56:04 2012
@@ -29,6 +29,7 @@ import org.apache.felix.useradmin.RoleFa
import org.apache.felix.useradmin.RoleRepositoryStore;
import org.apache.felix.useradmin.impl.role.RoleImpl;
import org.osgi.framework.Filter;
+import org.osgi.service.useradmin.Group;
import org.osgi.service.useradmin.Role;
import org.osgi.service.useradmin.UserAdminPermission;
@@ -44,50 +45,50 @@ public final class RoleRepository {
/**
* {@inheritDoc}
*/
- public void roleAdded(Role role) {
+ public void propertyAdded(Role role, Object key, Object value) {
Iterator iterator = createListenerIterator();
while (iterator.hasNext()) {
- ((RoleChangeListener) iterator.next()).roleAdded(role);
+ ((RoleChangeListener) iterator.next()).propertyAdded(role,
key, value);
}
}
/**
* {@inheritDoc}
*/
- public void roleRemoved(Role role) {
+ public void propertyChanged(Role role, Object key, Object oldValue,
Object newValue) {
Iterator iterator = createListenerIterator();
while (iterator.hasNext()) {
- ((RoleChangeListener) iterator.next()).roleRemoved(role);
+ ((RoleChangeListener) iterator.next()).propertyChanged(role,
key, oldValue, newValue);
}
}
/**
* {@inheritDoc}
*/
- public void propertyAdded(Role role, Object key, Object value) {
+ public void propertyRemoved(Role role, Object key) {
Iterator iterator = createListenerIterator();
while (iterator.hasNext()) {
- ((RoleChangeListener) iterator.next()).propertyAdded(role,
key, value);
+ ((RoleChangeListener) iterator.next()).propertyRemoved(role,
key);
}
}
/**
* {@inheritDoc}
*/
- public void propertyRemoved(Role role, Object key) {
+ public void roleAdded(Role role) {
Iterator iterator = createListenerIterator();
while (iterator.hasNext()) {
- ((RoleChangeListener) iterator.next()).propertyRemoved(role,
key);
+ ((RoleChangeListener) iterator.next()).roleAdded(role);
}
}
/**
* {@inheritDoc}
*/
- public void propertyChanged(Role role, Object key, Object oldValue,
Object newValue) {
+ public void roleRemoved(Role role) {
Iterator iterator = createListenerIterator();
while (iterator.hasNext()) {
- ((RoleChangeListener) iterator.next()).propertyChanged(role,
key, oldValue, newValue);
+ ((RoleChangeListener) iterator.next()).roleRemoved(role);
}
}
}
@@ -251,6 +252,9 @@ public final class RoleRepository {
try {
if (m_store.removeRole(role)) {
+ // FELIX-3755: Remove the role as (required)member from all
groups...
+ removeRoleFromAllGroups(role);
+
unwireChangeListener(role);
m_roleChangeReflector.roleRemoved(role);
@@ -278,7 +282,7 @@ public final class RoleRepository {
m_listeners.remove(listener);
}
- /**
+ /**
* Starts this repository.
*/
public void start() {
@@ -292,7 +296,7 @@ public final class RoleRepository {
e.printStackTrace();
}
}
-
+
/**
* Stops this repository, allowing it to clean up.
*/
@@ -304,7 +308,7 @@ public final class RoleRepository {
// Ignore; nothing we can do about this here...
}
}
-
+
/**
* Creates a new iterator for iterating over all listeners.
*
@@ -325,7 +329,7 @@ public final class RoleRepository {
securityManager.checkPermission(new
UserAdminPermission(UserAdminPermission.ADMIN, null));
}
}
-
+
/**
* Returns whether or not the given role is a predefined role.
* <p>
@@ -338,6 +342,39 @@ public final class RoleRepository {
private boolean isPredefinedRole(Role role) {
return Role.USER_ANYONE.equals(role.getName());
}
+
+ /**
+ * Removes a given role as (required)member from any groups it is member
of.
+ *
+ * @param removedRole the role that is removed from the store already,
cannot be <code>null</code>.
+ * @throws BackendException in case of problems accessing the store.
+ */
+ private void removeRoleFromAllGroups(Role removedRole) {
+ try {
+ Role[] roles = m_store.getAllRoles();
+ for (int i = 0; i < roles.length; i++) {
+ if (roles[i].getType() == Role.GROUP) {
+ Group group = (Group) roles[i];
+ // Don't check whether the given role is actually a
member
+ // of the group, but let the group itself figure this
out...
+ group.removeMember(removedRole);
+ }
+ }
+ }
+ catch (IOException e) {
+ throw new BackendException("Failed to get all roles!", e);
+ }
+ }
+
+ /**
+ * Unwires the given role to this repository so it no longer listens for
its changes.
+ *
+ * @param role the role to unwire, cannot be <code>null</code>.
+ * @throws IllegalArgumentException in case the given object was not a
{@link RoleImpl} instance.
+ */
+ private void unwireChangeListener(Object role) {
+ ((RoleImpl) role).setRoleChangeListener(null);
+ }
/**
* Wires the given role to this repository so it can listen for its
changes.
@@ -353,15 +390,4 @@ public final class RoleRepository {
}
return result;
}
-
- /**
- * Unwires the given role to this repository so it no longer listens for
its changes.
- *
- * @param role the role to unwire, cannot be <code>null</code>.
- * @throws IllegalArgumentException in case the given object was not a
{@link RoleImpl} instance.
- */
- private void unwireChangeListener(Object role) {
- RoleImpl result = (RoleImpl) role;
- result.setRoleChangeListener(null);
- }
}
Modified:
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/role/GroupImpl.java
URL:
http://svn.apache.org/viewvc/felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/role/GroupImpl.java?rev=1409119&r1=1409118&r2=1409119&view=diff
==============================================================================
---
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/role/GroupImpl.java
(original)
+++
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/role/GroupImpl.java
Wed Nov 14 08:56:04 2012
@@ -17,7 +17,6 @@
package org.apache.felix.useradmin.impl.role;
import java.util.Collection;
-import java.util.Dictionary;
import java.util.HashMap;
import java.util.Map;
@@ -53,29 +52,19 @@ public class GroupImpl extends UserImpl
}
/**
- * Creates a new {@link GroupImpl} instance of type {@link Role#GROUP}.
- *
- * @param name the name of this group role, cannot be <code>null</code> or
empty.
- */
- public GroupImpl(String name, Dictionary properties, Dictionary
credentials) {
- super(Role.GROUP, name, properties, credentials);
-
- m_members = new HashMap();
- m_requiredMembers = new HashMap();
- }
-
- /**
* {@inheritDoc}
*/
public boolean addMember(Role role) {
checkPermissions();
+ String name = role.getName();
+
Object result;
synchronized (m_lock) {
- if (m_members.containsKey(role.getName()) ||
m_requiredMembers.containsKey(role.getName())) {
+ if (m_members.containsKey(name) ||
m_requiredMembers.containsKey(name)) {
return false;
}
- result = m_members.put(role.getName(), role);
+ result = m_members.put(name, role);
}
if (result == null) {
@@ -92,12 +81,14 @@ public class GroupImpl extends UserImpl
public boolean addRequiredMember(Role role) {
checkPermissions();
+ String name = role.getName();
+
Object result;
synchronized (m_lock) {
- if (m_requiredMembers.containsKey(role.getName()) ||
m_members.containsKey(role.getName())) {
+ if (m_requiredMembers.containsKey(name) ||
m_members.containsKey(name)) {
return false;
}
- result = m_requiredMembers.put(role.getName(), role);
+ result = m_requiredMembers.put(name, role);
}
if (result == null) {
@@ -140,17 +131,19 @@ public class GroupImpl extends UserImpl
public boolean removeMember(Role role) {
checkPermissions();
+ String name = role.getName();
+
String key = null;
Object result = null;
synchronized (m_lock) {
- if (m_requiredMembers.containsKey(role.getName())) {
+ if (m_requiredMembers.containsKey(name)) {
key = REQUIRED_MEMBER;
- result = m_requiredMembers.remove(role.getName());
+ result = m_requiredMembers.remove(name);
}
- else if (m_members.containsKey(role.getName())) {
+ else if (m_members.containsKey(name)) {
key = BASIC_MEMBER;
- result = m_members.remove(role.getName());
+ result = m_members.remove(name);
}
}
Modified:
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/role/RoleImpl.java
URL:
http://svn.apache.org/viewvc/felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/role/RoleImpl.java?rev=1409119&r1=1409118&r2=1409119&view=diff
==============================================================================
---
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/role/RoleImpl.java
(original)
+++
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/role/RoleImpl.java
Wed Nov 14 08:56:04 2012
@@ -64,23 +64,6 @@ public class RoleImpl implements Seriali
}
/**
- * Creates a new {@link RoleImpl} instance with a given type, name and
properties.
- *
- * @param type the type of this role, should be {@link Role#ROLE}, {@link
Role#USER} or {@link Role#GROUP};
- * @param name the name of this role, cannot be <code>null</code> or empty;
- * @param properties the initial properties of this role, cannot be
<code>null</code>.
- */
- protected RoleImpl(int type, String name, Dictionary properties) {
- if (name == null || "".equals(name.trim())) {
- throw new IllegalArgumentException("Name cannot be null or
empty!");
- }
- m_type = type;
- m_name = name;
- m_properties = new ObservableProperties(null,
UserAdminPermission.CHANGE_PROPERTY, properties);
- m_properties.setDictionaryChangeListener(this);
- }
-
- /**
* {@inheritDoc}
*/
public final void entryAdded(Object key, Object value) {
Modified:
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/role/UserImpl.java
URL:
http://svn.apache.org/viewvc/felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/role/UserImpl.java?rev=1409119&r1=1409118&r2=1409119&view=diff
==============================================================================
---
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/role/UserImpl.java
(original)
+++
felix/trunk/useradmin/useradmin/src/main/java/org/apache/felix/useradmin/impl/role/UserImpl.java
Wed Nov 14 08:56:04 2012
@@ -55,18 +55,6 @@ public class UserImpl extends RoleImpl i
}
/**
- * Creates a new {@link UserImpl} instance with type {@link Role#USER}.
- *
- * @param name the name of this user role, cannot be <code>null</code> or
empty.
- */
- protected UserImpl(int type, String name, Dictionary properties,
Dictionary credentials) {
- super(type, name, properties);
-
- m_credentials = new
ObservableProperties(UserAdminPermission.GET_CREDENTIAL,
UserAdminPermission.CHANGE_CREDENTIAL, credentials);
- m_credentials.setDictionaryChangeListener(this);
- }
-
- /**
* {@inheritDoc}
*/
public Dictionary getCredentials() {
@@ -89,7 +77,7 @@ public class UserImpl extends RoleImpl i
} else if (value instanceof String) {
s2 = (String) value;
} else {
- // Not a string or a byte-array!
+ // Not a string, nor a byte-array!
return false;
}
@@ -102,7 +90,7 @@ public class UserImpl extends RoleImpl i
} else if (value instanceof String) {
b2 = ((String) value).getBytes();
} else {
- // Not a string or a byte-array!
+ // Not a string, nor a byte-array!
return false;
}
Modified:
felix/trunk/useradmin/useradmin/src/test/java/org/apache/felix/useradmin/impl/UserAdminImplTest.java
URL:
http://svn.apache.org/viewvc/felix/trunk/useradmin/useradmin/src/test/java/org/apache/felix/useradmin/impl/UserAdminImplTest.java?rev=1409119&r1=1409118&r2=1409119&view=diff
==============================================================================
---
felix/trunk/useradmin/useradmin/src/test/java/org/apache/felix/useradmin/impl/UserAdminImplTest.java
(original)
+++
felix/trunk/useradmin/useradmin/src/test/java/org/apache/felix/useradmin/impl/UserAdminImplTest.java
Wed Nov 14 08:56:04 2012
@@ -245,19 +245,6 @@ public class UserAdminImplTest extends T
}
/**
- * Tests that creating a role with an invalid type does not succeed and
yields an exception.
- */
- public void testCreateInvalidRoleTypeFail() {
- try {
- m_userAdmin.createRole("role1", Role.ROLE);
-
- fail("Expected an IllegalArgumentException!");
- } catch (IllegalArgumentException e) {
- // Ok; expected
- }
- }
-
- /**
* Tests that creating a role without a name does not succeed and yields
an exception.
*/
public void testCreateInvalidRoleNameFail() {
@@ -271,11 +258,11 @@ public class UserAdminImplTest extends T
}
/**
- * Tests that creating a role without a name does not succeed and yields
an exception.
+ * Tests that creating a role with an invalid type does not succeed and
yields an exception.
*/
- public void testCreateRoleWithEmptyNameFail() {
+ public void testCreateInvalidRoleTypeFail() {
try {
- m_userAdmin.createRole("", Role.USER);
+ m_userAdmin.createRole("role1", Role.ROLE);
fail("Expected an IllegalArgumentException!");
} catch (IllegalArgumentException e) {
@@ -306,6 +293,19 @@ public class UserAdminImplTest extends T
}
/**
+ * Tests that creating a role without a name does not succeed and yields
an exception.
+ */
+ public void testCreateRoleWithEmptyNameFail() {
+ try {
+ m_userAdmin.createRole("", Role.USER);
+
+ fail("Expected an IllegalArgumentException!");
+ } catch (IllegalArgumentException e) {
+ // Ok; expected
+ }
+ }
+
+ /**
* Tests that creating a {@link UserAdminImpl} with a null event
dispatcher fails.
*/
public void testCreateUserAdminImplWithNullDispatcherFail() {
@@ -690,6 +690,75 @@ public class UserAdminImplTest extends T
}
/**
+ * Tests that remove of a role also removes that role as member from any
group (FELIX-3755).
+ */
+ public void testRemoveRoleRemovesItAsGroupMemberOk() {
+ Role user1 = m_userAdmin.createRole("user1", Role.USER);
+ Role user2 = m_userAdmin.createRole("user2", Role.USER);
+
+ Group group1 = (Group) m_userAdmin.createRole("group1", Role.GROUP);
+ group1.addMember(user1);
+
+ Group group2 = (Group) m_userAdmin.createRole("group2", Role.GROUP);
+ group2.addMember(user1);
+ group2.addMember(user2);
+
+ // Remove user...
+ m_userAdmin.removeRole(user1.getName());
+
+ // Retrieve an up-to-date instance of the first group...
+ group1 = (Group) m_userAdmin.getRole("group1");
+ assertNull(group1.getMembers());
+
+ // Retrieve an up-to-date instance of the second group...
+ group2 = (Group) m_userAdmin.getRole("group2");
+
+ Role[] members = group2.getMembers();
+ assertNotNull(members);
+ assertEquals(1, members.length);
+ assertEquals(user2, members[0]);
+ }
+
+ /**
+ * Tests that remove of a role also removes that role as required member
from any group (FELIX-3755).
+ */
+ public void testRemoveRoleRemovesItAsRequiredGroupMemberOk() {
+ Role user1 = m_userAdmin.createRole("user1", Role.USER);
+ Role user2 = m_userAdmin.createRole("user2", Role.USER);
+
+ Group group1 = (Group) m_userAdmin.createRole("group1", Role.GROUP);
+ group1.addRequiredMember(user1);
+ group1.addMember(user2);
+
+ Group group2 = (Group) m_userAdmin.createRole("group2", Role.GROUP);
+ group2.addRequiredMember(user1);
+ group2.addRequiredMember(user2);
+
+ // Remove user...
+ m_userAdmin.removeRole(user1.getName());
+
+ // Retrieve an up-to-date instance of the group...
+ group1 = (Group) m_userAdmin.getRole("group1");
+
+ assertNull(group1.getRequiredMembers());
+
+ Role[] members = group1.getMembers();
+ assertNotNull(members);
+ assertEquals(1, members.length);
+ assertEquals(user2, members[0]);
+
+ // Retrieve an up-to-date instance of the group...
+ group2 = (Group) m_userAdmin.getRole("group2");
+
+ assertNull(group2.getMembers());
+
+ members = group2.getRequiredMembers();
+ assertNotNull(members);
+ assertEquals(1, members.length);
+ assertEquals(user2, members[0]);
+ }
+
+ /**
* Tests that remove a credential of a user works.
*/
public void testRemoveUserCredentialOk() {
