diff --git a/dspace-api/src/test/java/org/dspace/content/CollectionTest.java b/dspace-api/src/test/java/org/dspace/content/CollectionTest.java index a988abdda1..2e51fb908b 100644 --- a/dspace-api/src/test/java/org/dspace/content/CollectionTest.java +++ b/dspace-api/src/test/java/org/dspace/content/CollectionTest.java @@ -28,11 +28,9 @@ import java.util.Iterator; import java.util.List; import java.util.UUID; -import mockit.Mock; -import mockit.MockUp; import org.apache.logging.log4j.Logger; -import org.dspace.app.util.AuthorizeUtil; import org.dspace.authorize.AuthorizeException; +import org.dspace.authorize.factory.AuthorizeServiceFactory; import org.dspace.authorize.service.AuthorizeService; import org.dspace.core.Constants; import org.dspace.core.Context; @@ -95,11 +93,14 @@ public class CollectionTest extends AbstractDSpaceObjectTest { // Initialize our spy of the autowired (global) authorizeService bean. // This allows us to customize the bean's method return values in tests below authorizeServiceSpy = spy(authorizeService); - // "Wire" our spy to be used by the current loaded CommunityService, CollectionService & ItemService + // "Wire" our spy to be used by the current loaded object services // (To ensure these services use the spy instead of the real service) ReflectionTestUtils.setField(communityService, "authorizeService", authorizeServiceSpy); ReflectionTestUtils.setField(collectionService, "authorizeService", authorizeServiceSpy); ReflectionTestUtils.setField(itemService, "authorizeService", authorizeServiceSpy); + // Also wire into current AuthorizeServiceFactory, as that is used for some checks (e.g. AuthorizeUtil) + ReflectionTestUtils.setField(AuthorizeServiceFactory.getInstance(), "authorizeService", + authorizeServiceSpy); } catch (AuthorizeException ex) { log.error("Authorization Error in init", ex); fail("Authorization Error in init: " + ex.getMessage()); @@ -378,13 +379,8 @@ public class CollectionTest extends AbstractDSpaceObjectTest { */ @Test public void testCreateWorkflowGroupAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil & allow permissions - new MockUp() { - @Mock - public void authorizeManageWorkflowsGroup(Context context, Collection collection) { - return; // allow permissions by not throwing an exception - } - }; + // Allow Collection ADMIN (to manage workflow group) + doNothing().when(authorizeServiceSpy).authorizeAction(context, collection, Constants.ADMIN); int step = 1; Group result = collectionService.createWorkflowGroup(context, collection, step); @@ -396,15 +392,6 @@ public class CollectionTest extends AbstractDSpaceObjectTest { */ @Test(expected = AuthorizeException.class) public void testCreateWorkflowGroupNoAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil & disallow permissions - new MockUp() { - @Mock - public void authorizeManageWorkflowsGroup(Context context, Collection collection) - throws AuthorizeException { - throw new AuthorizeException(); - } - }; - int step = 1; Group result = collectionService.createWorkflowGroup(context, collection, step); fail("Exception expected"); @@ -462,13 +449,8 @@ public class CollectionTest extends AbstractDSpaceObjectTest { */ @Test public void testCreateSubmittersAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil & allow permissions - new MockUp() { - @Mock - public void authorizeManageSubmittersGroup(Context context, Collection collection) { - return; // allow permissions by not throwing an exception - } - }; + // Allow Collection ADMIN (to manage submitter group) + doNothing().when(authorizeServiceSpy).authorizeAction(context, collection, Constants.ADMIN); Group result = collectionService.createSubmitters(context, collection); assertThat("testCreateSubmittersAuth 0", result, notNullValue()); @@ -479,15 +461,6 @@ public class CollectionTest extends AbstractDSpaceObjectTest { */ @Test(expected = AuthorizeException.class) public void testCreateSubmittersNoAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil & disallow permissions - new MockUp() { - @Mock - public void authorizeManageSubmittersGroup(Context context, Collection collection) - throws AuthorizeException { - throw new AuthorizeException(); - } - }; - Group result = collectionService.createSubmitters(context, collection); fail("Exception expected"); } @@ -497,13 +470,8 @@ public class CollectionTest extends AbstractDSpaceObjectTest { */ @Test public void testRemoveSubmittersAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil & allow permissions - new MockUp() { - @Mock - public void authorizeManageSubmittersGroup(Context context, Collection collection) { - return; // allow permissions by not throwing an exception - } - }; + // Allow Collection ADMIN (to manage submitter group) + doNothing().when(authorizeServiceSpy).authorizeAction(context, collection, Constants.ADMIN); collectionService.removeSubmitters(context, collection); assertThat("testRemoveSubmittersAuth 0", collection.getSubmitters(), nullValue()); @@ -514,15 +482,6 @@ public class CollectionTest extends AbstractDSpaceObjectTest { */ @Test(expected = AuthorizeException.class) public void testRemoveSubmittersNoAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil & disallow permissions - new MockUp() { - @Mock - public void authorizeManageSubmittersGroup(Context context, Collection collection) - throws AuthorizeException { - throw new AuthorizeException(); - } - }; - collectionService.removeSubmitters(context, collection); fail("Exception expected"); } @@ -540,13 +499,8 @@ public class CollectionTest extends AbstractDSpaceObjectTest { */ @Test public void testCreateAdministratorsAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil & allow permissions - new MockUp() { - @Mock - public void authorizeManageAdminGroup(Context context, Collection collection) { - return; // allow permissions by not throwing an exception - } - }; + // Allow Collection ADMIN (to manage admin group) + doNothing().when(authorizeServiceSpy).authorizeAction(context, collection, Constants.ADMIN); Group result = collectionService.createAdministrators(context, collection); assertThat("testCreateAdministratorsAuth 0", result, notNullValue()); @@ -557,15 +511,6 @@ public class CollectionTest extends AbstractDSpaceObjectTest { */ @Test(expected = AuthorizeException.class) public void testCreateAdministratorsNoAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil & disallow permissions - new MockUp() { - @Mock - public void authorizeManageAdminGroup(Context context, Collection collection) - throws AuthorizeException { - throw new AuthorizeException(); - } - }; - Group result = collectionService.createAdministrators(context, collection); fail("Exception expected"); } @@ -575,13 +520,8 @@ public class CollectionTest extends AbstractDSpaceObjectTest { */ @Test public void testRemoveAdministratorsAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil & allow permissions - new MockUp() { - @Mock - public void authorizeRemoveAdminGroup(Context context, Collection collection) { - return; // allow permissions by not throwing an exception - } - }; + // Allow parent Community ADMIN (only Community Admins can delete a Collection Admin group) + doNothing().when(authorizeServiceSpy).authorizeAction(context, owningCommunity, Constants.ADMIN); // Ensure admin group is created first context.turnOffAuthorisationSystem(); @@ -599,14 +539,6 @@ public class CollectionTest extends AbstractDSpaceObjectTest { */ @Test(expected = AuthorizeException.class) public void testRemoveAdministratorsNoAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil & disallow permissions - new MockUp() { - @Mock - public void authorizeRemoveAdminGroup(Context context, Collection collection) throws AuthorizeException { - throw new AuthorizeException(); - } - }; - // Ensure admin group is created first context.turnOffAuthorisationSystem(); Group result = collectionService.createAdministrators(context, collection); @@ -679,13 +611,8 @@ public class CollectionTest extends AbstractDSpaceObjectTest { */ @Test public void testCreateTemplateItemAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil & allow permissions - new MockUp() { - @Mock - public void authorizeManageTemplateItem(Context context, Collection collection) { - return; // allow permissions by not throwing an exception - } - }; + // Allow Collection ADMIN (to manage template item) + doNothing().when(authorizeServiceSpy).authorizeAction(context, collection, Constants.ADMIN); itemService.createTemplateItem(context, collection); assertThat("testCreateTemplateItemAuth 0", collection.getTemplateItem(), notNullValue()); @@ -696,15 +623,6 @@ public class CollectionTest extends AbstractDSpaceObjectTest { */ @Test(expected = AuthorizeException.class) public void testCreateTemplateItemNoAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil & disallow permissions - new MockUp() { - @Mock - public void authorizeManageTemplateItem(Context context, Collection collection) - throws AuthorizeException { - throw new AuthorizeException(); - } - }; - itemService.createTemplateItem(context, collection); fail("Exception expected"); } @@ -714,13 +632,8 @@ public class CollectionTest extends AbstractDSpaceObjectTest { */ @Test public void testRemoveTemplateItemAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil & allow permissions - new MockUp() { - @Mock - public void authorizeManageTemplateItem(Context context, Collection collection) { - return; // allow permissions by not throwing an exception - } - }; + // Allow Collection ADMIN (to manage template item) + doNothing().when(authorizeServiceSpy).authorizeAction(context, collection, Constants.ADMIN); collectionService.removeTemplateItem(context, collection); assertThat("testRemoveTemplateItemAuth 0", collection.getTemplateItem(), nullValue()); @@ -731,15 +644,6 @@ public class CollectionTest extends AbstractDSpaceObjectTest { */ @Test(expected = AuthorizeException.class) public void testRemoveTemplateItemNoAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil & disallow permissions - new MockUp() { - @Mock - public void authorizeManageTemplateItem(Context context, Collection collection) - throws AuthorizeException { - throw new AuthorizeException(); - } - }; - collectionService.removeTemplateItem(context, collection); fail("Exception expected"); } @@ -1069,13 +973,8 @@ public class CollectionTest extends AbstractDSpaceObjectTest { public void testDeleteAuth() throws Exception { // Allow Collection WRITE perms doNothing().when(authorizeServiceSpy).authorizeAction(context, collection, Constants.WRITE, true); - // Use JMockit to mock the AuthorizeUtil & allow permissions - new MockUp() { - @Mock - public void authorizeManageTemplateItem(Context context, Collection collection) { - return; // allow permissions by not throwing an exception - } - }; + // Allow Collection ADMIN perms + doNothing().when(authorizeServiceSpy).authorizeAction(context, collection, Constants.ADMIN); UUID id = collection.getID(); collectionService.delete(context, collection); @@ -1092,26 +991,6 @@ public class CollectionTest extends AbstractDSpaceObjectTest { fail("Exception expected"); } - /** - * Test of delete method, of class Collection. - */ - @Test(expected = AuthorizeException.class) - public void testDeleteNoAuth2() throws Exception { - // Allow Collection WRITE perms - doThrow(new AuthorizeException()).when(authorizeServiceSpy) - .authorizeAction(context, collection, Constants.WRITE, true); - // Use JMockit to mock the AuthorizeUtil & allow permissions - new MockUp() { - @Mock - public void authorizeManageTemplateItem(Context context, Collection collection) { - return; // allow permissions by not throwing an exception - } - }; - - collectionService.delete(context, collection); - fail("Exception expected"); - } - /** * Test of getCommunities method, of class Collection. */ diff --git a/dspace-api/src/test/java/org/dspace/content/CommunityTest.java b/dspace-api/src/test/java/org/dspace/content/CommunityTest.java index ca25cc4ae4..0bd9e39fdf 100644 --- a/dspace-api/src/test/java/org/dspace/content/CommunityTest.java +++ b/dspace-api/src/test/java/org/dspace/content/CommunityTest.java @@ -29,11 +29,9 @@ import java.sql.SQLException; import java.util.List; import java.util.UUID; -import mockit.Mock; -import mockit.MockUp; import org.apache.logging.log4j.Logger; -import org.dspace.app.util.AuthorizeUtil; import org.dspace.authorize.AuthorizeException; +import org.dspace.authorize.factory.AuthorizeServiceFactory; import org.dspace.authorize.service.AuthorizeService; import org.dspace.core.Constants; import org.dspace.core.Context; @@ -89,11 +87,14 @@ public class CommunityTest extends AbstractDSpaceObjectTest { // Initialize our spy of the autowired (global) authorizeService bean. // This allows us to customize the bean's method return values in tests below authorizeServiceSpy = spy(authorizeService); - // "Wire" our spy to be used by the current loaded CommunityService, CollectionService & ItemService + // "Wire" our spy to be used by the current loaded object services // (To ensure both these services use the spy instead of the real service) ReflectionTestUtils.setField(communityService, "authorizeService", authorizeServiceSpy); ReflectionTestUtils.setField(collectionService, "authorizeService", authorizeServiceSpy); ReflectionTestUtils.setField(itemService, "authorizeService", authorizeServiceSpy); + // Also wire into current AuthorizeServiceFactory, as that is used for some checks (e.g. AuthorizeUtil) + ReflectionTestUtils.setField(AuthorizeServiceFactory.getInstance(), "authorizeService", + authorizeServiceSpy); } catch (AuthorizeException ex) { log.error("Authorization Error in init", ex); fail("Authorization Error in init: " + ex.getMessage()); @@ -411,13 +412,8 @@ public class CommunityTest extends AbstractDSpaceObjectTest { */ @Test public void testCreateAdministratorsAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil which is used by createAdministrators() to check permissions - new MockUp() { - @Mock - public void authorizeManageAdminGroup(Context context, Community community) { - return; // allow permissions by not throwing an exception - } - }; + // Allow Community ADMIN perms + doNothing().when(authorizeServiceSpy).authorizeAction(context, c, Constants.ADMIN); Group result = communityService.createAdministrators(context, c); assertThat("testCreateAdministratorsAuth 0", c.getAdministrators(), notNullValue()); @@ -429,14 +425,6 @@ public class CommunityTest extends AbstractDSpaceObjectTest { */ @Test(expected = AuthorizeException.class) public void testCreateAdministratorsNoAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil which is used by createAdministrators() to check permissions - new MockUp() { - @Mock - public void authorizeManageAdminGroup(Context context, Community community) throws AuthorizeException { - throw new AuthorizeException("fake error message"); - } - }; - Group result = communityService.createAdministrators(context, c); fail("Exception should have been thrown"); } @@ -447,13 +435,8 @@ public class CommunityTest extends AbstractDSpaceObjectTest { */ @Test public void testRemoveAdministratorsAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil which is used by createAdministrators() to check permissions - new MockUp() { - @Mock - public void authorizeRemoveAdminGroup(Context context, Community community) { - return; // allow permissions by not throwing an exception - } - }; + // Allow Full ADMIN perms (Community Admins cannot delete their Admin Group) + when(authorizeServiceSpy.isAdmin(context)).thenReturn(true); // Ensure admin group is created first context.turnOffAuthorisationSystem(); @@ -471,13 +454,8 @@ public class CommunityTest extends AbstractDSpaceObjectTest { */ @Test(expected = AuthorizeException.class) public void testRemoveAdministratorsNoAuth() throws Exception { - // Use JMockit to mock the AuthorizeUtil which is used by createAdministrators() to check permissions - new MockUp() { - @Mock - public void authorizeRemoveAdminGroup(Context context, Community community) throws AuthorizeException { - throw new AuthorizeException("fake error message"); - } - }; + // Allow Community ADMIN perms (Community Admins cannot delete their Admin Group) + doNothing().when(authorizeServiceSpy).authorizeAction(context, c, Constants.ADMIN); // Ensure admin group is created first context.turnOffAuthorisationSystem(); @@ -736,20 +714,6 @@ public class CommunityTest extends AbstractDSpaceObjectTest { */ @Test public void testRemoveCollectionAuth() throws Exception { - // Allow current Community REMOVE perms (to remove Collection from Community & delete) - doNothing().when(authorizeServiceSpy).authorizeAction(context, c, Constants.REMOVE); - // Allow collection WRITE perms (needed to remove Logo before Collection can be deleted) - doNothing().when(authorizeServiceSpy) - .authorizeAction(any(Context.class), any(Collection.class), eq(Constants.WRITE), eq(true)); - // Use JMockit to mock the AuthorizeUtil - new MockUp() { - // Allow Collection ManageTemplateItem perms (needed to remove Item Template before Collection is deleted) - @Mock - public void authorizeManageTemplateItem(Context context, Collection collection) { - return; // allow permissions by not throwing an exception - } - }; - // Turn off authorization temporarily to create a new Collection context.turnOffAuthorisationSystem(); Collection col = collectionService.create(context, c); @@ -758,6 +722,13 @@ public class CommunityTest extends AbstractDSpaceObjectTest { assertTrue("testRemoveCollectionAuth 1", c.getCollections().size() == 1); assertThat("testRemoveCollectionAuth 2", c.getCollections().get(0), equalTo(col)); + // Allow current Community REMOVE perms (to remove Collection from Community & delete) + doNothing().when(authorizeServiceSpy).authorizeAction(context, c, Constants.REMOVE); + // Allow collection WRITE perms (needed to remove Logo before Collection can be deleted) + doNothing().when(authorizeServiceSpy).authorizeAction(context, col, Constants.WRITE,true); + // Allow current Community ADMIN perms (to remove Collection from Community & delete) + doNothing().when(authorizeServiceSpy).authorizeAction(context, col, Constants.ADMIN); + // Note that this will *also* delete the collection (hence the extra permissions provided above) communityService.removeCollection(context, c, col); assertThat("testRemoveCollectionAuth 3", c.getCollections(), notNullValue()); @@ -876,6 +847,9 @@ public class CommunityTest extends AbstractDSpaceObjectTest { // Allow current Collection DELETE perms (needed to delete a Collection) doNothing().when(authorizeServiceSpy) .authorizeAction(any(Context.class), any(Collection.class), eq(Constants.DELETE), eq(true)); + // Allow current Collection ADMIN perms (needed to delete a Collection) + doNothing().when(authorizeServiceSpy) + .authorizeAction(any(Context.class), any(Collection.class), eq(Constants.ADMIN)); // Allow current Item WRITE perms (needed to remove identifiers from an Item prior to deletion) doNothing().when(authorizeServiceSpy) .authorizeAction(any(Context.class), any(Item.class), eq(Constants.WRITE), eq(true)); @@ -885,14 +859,6 @@ public class CommunityTest extends AbstractDSpaceObjectTest { // Allow current Item REMOVE perms (needed to delete Item) doNothing().when(authorizeServiceSpy) .authorizeAction(any(Context.class), any(Item.class), eq(Constants.REMOVE), eq(true)); - // Use JMockit to mock the AuthorizeUtil - new MockUp() { - // Allow Collection ManageTemplateItem perms (needed to delete a collection) - @Mock - public void authorizeManageTemplateItem(Context context, Collection collection) { - return; // allow permissions by not throwing an exception - } - }; // Create a dummy Community hierarchy to test delete with // Turn off authorization temporarily to create some test objects. diff --git a/dspace-api/src/test/java/org/dspace/content/ItemTest.java b/dspace-api/src/test/java/org/dspace/content/ItemTest.java index 549f4c0075..c996ef7b16 100644 --- a/dspace-api/src/test/java/org/dspace/content/ItemTest.java +++ b/dspace-api/src/test/java/org/dspace/content/ItemTest.java @@ -33,13 +33,11 @@ import java.util.Iterator; import java.util.List; import java.util.UUID; -import mockit.Mock; -import mockit.MockUp; import org.apache.commons.lang3.time.DateUtils; import org.apache.logging.log4j.Logger; -import org.dspace.app.util.AuthorizeUtil; import org.dspace.authorize.AuthorizeException; import org.dspace.authorize.ResourcePolicy; +import org.dspace.authorize.factory.AuthorizeServiceFactory; import org.dspace.authorize.service.AuthorizeService; import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.service.BitstreamFormatService; @@ -121,6 +119,9 @@ public class ItemTest extends AbstractDSpaceObjectTest { ReflectionTestUtils.setField(workspaceItemService, "authorizeService", authorizeServiceSpy); ReflectionTestUtils.setField(bundleService, "authorizeService", authorizeServiceSpy); ReflectionTestUtils.setField(bitstreamService, "authorizeService", authorizeServiceSpy); + // Also wire into current AuthorizeServiceFactory, as that is used for some checks (e.g. AuthorizeUtil) + ReflectionTestUtils.setField(AuthorizeServiceFactory.getInstance(), "authorizeService", + authorizeServiceSpy); } catch (AuthorizeException ex) { log.error("Authorization Error in init", ex); fail("Authorization Error in init: " + ex.getMessage()); @@ -1124,14 +1125,8 @@ public class ItemTest extends AbstractDSpaceObjectTest { public void testWithdrawAuth() throws Exception { // Allow Item WRITE perms doNothing().when(authorizeServiceSpy).authorizeAction(context, it, Constants.WRITE); - - // Use JMockit to mock the AuthorizeUtil & allow permissions - new MockUp() { - @Mock - public void authorizeWithdrawItem(Context context, Item item) { - return; // allow permissions by not throwing an exception - } - }; + // Allow Collection ADMIN perms + when(authorizeServiceSpy.authorizeActionBoolean(context, collection, Constants.ADMIN)).thenReturn(true); itemService.withdraw(context, it); assertTrue("testWithdrawAuth 0", it.isWithdrawn()); @@ -1153,14 +1148,8 @@ public class ItemTest extends AbstractDSpaceObjectTest { public void testReinstateAuth() throws Exception { // Allow Item WRITE perms doNothing().when(authorizeServiceSpy).authorizeAction(context, it, Constants.WRITE); - - // Use JMockit to mock the AuthorizeUtil & allow permissions - new MockUp() { - @Mock - public void authorizeReinstateItem(Context context, Item item) { - return; // allow permissions by not throwing an exception - } - }; + // Allow Collection ADD perms (needed to reinstate) + doNothing().when(authorizeServiceSpy).authorizeAction(context, collection, Constants.ADD); // initialize item as withdrawn context.turnOffAuthorisationSystem(); @@ -1199,8 +1188,6 @@ public class ItemTest extends AbstractDSpaceObjectTest { doNothing().when(authorizeServiceSpy).authorizeAction(context, item, Constants.REMOVE, true); // Allow Item DELETE perms doNothing().when(authorizeServiceSpy).authorizeAction(context, item, Constants.DELETE); - // Allow Item WRITE perms - doNothing().when(authorizeServiceSpy).authorizeAction(context, item, Constants.WRITE); UUID id = item.getID(); itemService.delete(context, item);