From ec30c2506ec6206d652dfbf2810b8eaad5318513 Mon Sep 17 00:00:00 2001 From: aboeckle <alexander.boeckle@frentix.com> Date: Thu, 20 Aug 2020 18:17:48 +0200 Subject: [PATCH] OO-4844 Ignore guest related consents --- .../disclaimer/CourseDisclaimerManager.java | 5 +- .../manager/CourseDisclaimerManagerImpl.java | 9 ++-- .../ui/CourseDisclaimerConsentController.java | 2 +- .../olat/course/run/RunMainController.java | 2 +- .../CourseDisclaimerManagerTest.java | 52 +++++++++++-------- .../lifecycle/UserLifecycleManagerTest.java | 5 +- 6 files changed, 43 insertions(+), 32 deletions(-) diff --git a/src/main/java/org/olat/course/disclaimer/CourseDisclaimerManager.java b/src/main/java/org/olat/course/disclaimer/CourseDisclaimerManager.java index 32978bf8ec8..84b9e7c5bd4 100644 --- a/src/main/java/org/olat/course/disclaimer/CourseDisclaimerManager.java +++ b/src/main/java/org/olat/course/disclaimer/CourseDisclaimerManager.java @@ -23,6 +23,7 @@ import java.util.List; import org.olat.basesecurity.IdentityRef; import org.olat.core.id.Identity; +import org.olat.core.id.Roles; import org.olat.repository.RepositoryEntry; import org.olat.repository.RepositoryEntryRef; @@ -53,7 +54,7 @@ public interface CourseDisclaimerManager { * @param disc1Accepted * @param disc2Accepted */ - public void acceptDisclaimer(RepositoryEntry repositoryEntry, Identity identity, boolean disc1Accepted, boolean disc2Accepted); + public void acceptDisclaimer(RepositoryEntry repositoryEntry, Identity identity, Roles roles, boolean disc1Accepted, boolean disc2Accepted); /** * Deletes all disclaimer consent database entries related to a repository entry and a list of users @@ -86,7 +87,7 @@ public interface CourseDisclaimerManager { * @param identityRef * @return boolean */ - public boolean isAccessGranted(RepositoryEntry repositoryEntry, IdentityRef identityRef); + public boolean isAccessGranted(RepositoryEntry repositoryEntry, IdentityRef identityRef, Roles roles); /** * Returns whether any disclaimer has been accepted yet diff --git a/src/main/java/org/olat/course/disclaimer/manager/CourseDisclaimerManagerImpl.java b/src/main/java/org/olat/course/disclaimer/manager/CourseDisclaimerManagerImpl.java index f8f172fe1f1..a3792fcac5a 100644 --- a/src/main/java/org/olat/course/disclaimer/manager/CourseDisclaimerManagerImpl.java +++ b/src/main/java/org/olat/course/disclaimer/manager/CourseDisclaimerManagerImpl.java @@ -26,6 +26,7 @@ import org.olat.basesecurity.BaseSecurityManager; import org.olat.basesecurity.IdentityRef; import org.olat.core.commons.persistence.DB; import org.olat.core.id.Identity; +import org.olat.core.id.Roles; import org.olat.core.logging.Tracing; import org.olat.course.CourseFactory; import org.olat.course.config.CourseConfig; @@ -72,8 +73,8 @@ public class CourseDisclaimerManagerImpl implements CourseDisclaimerManager, Use } @Override - public void acceptDisclaimer(RepositoryEntry repositoryEntry, Identity identitiy, boolean disc1Accepted, boolean disc2Accepted) { - if (baseSecurityManager.getRoles(identitiy).isGuestOnly()) { + public void acceptDisclaimer(RepositoryEntry repositoryEntry, Identity identitiy, Roles roles, boolean disc1Accepted, boolean disc2Accepted) { + if (roles.isGuestOnly()) { return; } @@ -94,7 +95,7 @@ public class CourseDisclaimerManagerImpl implements CourseDisclaimerManager, Use } @Override - public boolean isAccessGranted(RepositoryEntry repositoryEntry, IdentityRef identitiyRef) { + public boolean isAccessGranted(RepositoryEntry repositoryEntry, IdentityRef identitiyRef, Roles roles) { CourseConfig courseConfig = CourseFactory.loadCourse(repositoryEntry.getOlatResource().getResourceableId()).getCourseConfig(); boolean accessGranted = true; @@ -115,7 +116,7 @@ public class CourseDisclaimerManagerImpl implements CourseDisclaimerManager, Use accessGranted &= false; } } - if (baseSecurityManager.getRoles(identitiyRef).isGuestOnly()) { + if (roles.isGuestOnly()) { accessGranted &= false; } } diff --git a/src/main/java/org/olat/course/disclaimer/ui/CourseDisclaimerConsentController.java b/src/main/java/org/olat/course/disclaimer/ui/CourseDisclaimerConsentController.java index e2cbbcebee5..d09692bc14b 100644 --- a/src/main/java/org/olat/course/disclaimer/ui/CourseDisclaimerConsentController.java +++ b/src/main/java/org/olat/course/disclaimer/ui/CourseDisclaimerConsentController.java @@ -68,7 +68,7 @@ public class CourseDisclaimerConsentController extends FormBasicController { @Override protected void formOK(UserRequest ureq) { - disclaimerManager.acceptDisclaimer(repositoryEntry, getIdentity(), courseConfig.isDisclaimerEnabled(1), courseConfig.isDisclaimerEnabled(2)); + disclaimerManager.acceptDisclaimer(repositoryEntry, getIdentity(), ureq.getUserSession().getRoles(), courseConfig.isDisclaimerEnabled(1), courseConfig.isDisclaimerEnabled(2)); fireEvent(ureq, Event.DONE_EVENT); } diff --git a/src/main/java/org/olat/course/run/RunMainController.java b/src/main/java/org/olat/course/run/RunMainController.java index a3627ed18f8..9ab3b232cb9 100644 --- a/src/main/java/org/olat/course/run/RunMainController.java +++ b/src/main/java/org/olat/course/run/RunMainController.java @@ -282,7 +282,7 @@ public class RunMainController extends MainLayoutBasicController implements Gene // if a disclaimer is enabled, show it first if (courseModule.isDisclaimerEnabled() && course.getCourseEnvironment().getCourseConfig().isDisclaimerEnabled() && - !disclaimerManager.isAccessGranted(courseRepositoryEntry, getIdentity())) { + !disclaimerManager.isAccessGranted(courseRepositoryEntry, getIdentity(), ureq.getUserSession().getRoles())) { disclaimerController = new CourseDisclaimerConsentController(ureq, getWindowControl(), courseRepositoryEntry); listenTo(disclaimerController); coursemain.put("coursemain", disclaimerController.getInitialComponent()); diff --git a/src/test/java/org/olat/course/disclaimer/CourseDisclaimerManagerTest.java b/src/test/java/org/olat/course/disclaimer/CourseDisclaimerManagerTest.java index ca84ca6dcb6..a4ce814d847 100644 --- a/src/test/java/org/olat/course/disclaimer/CourseDisclaimerManagerTest.java +++ b/src/test/java/org/olat/course/disclaimer/CourseDisclaimerManagerTest.java @@ -32,10 +32,12 @@ import java.util.List; import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import org.olat.basesecurity.BaseSecurityManager; import org.olat.basesecurity.OrganisationService; import org.olat.core.commons.persistence.DB; import org.olat.core.id.Identity; import org.olat.core.id.Organisation; +import org.olat.core.id.Roles; import org.olat.course.CourseFactory; import org.olat.course.CourseModule; import org.olat.course.ICourse; @@ -67,7 +69,9 @@ public class CourseDisclaimerManagerTest extends OlatTestCase { private String disclaimer2Label2 = "Course 2 Label 2"; private Identity id1; + private Roles roles1; private Identity id2; + private Roles roles2; private RepositoryEntry repositoryEntry; private ICourse course; @@ -84,6 +88,8 @@ public class CourseDisclaimerManagerTest extends OlatTestCase { private OLATResourceManager resourceManager; @Autowired private RepositoryService repositoryService; + @Autowired + private BaseSecurityManager baseSecurityManager; @Before @@ -98,7 +104,9 @@ public class CourseDisclaimerManagerTest extends OlatTestCase { "Test custom course disclaimer"); id1 = JunitTestHelper.createAndPersistIdentityAsUser("id1"); + roles1 = baseSecurityManager.getRoles(id1); id2 = JunitTestHelper.createAndPersistIdentityAsUser("id2"); + roles2 = baseSecurityManager.getRoles(id2); dbInstance.commitAndCloseSession(); } @@ -155,13 +163,13 @@ public class CourseDisclaimerManagerTest extends OlatTestCase { Assert.assertFalse(courseDisclaimerManager.hasAnyConsent(repositoryEntry)); - courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id1, true, true); - courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id2, true, false); + courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id1, roles1, true, true); + courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id2, roles2, true, false); dbInstance.commitAndCloseSession(); - Assert.assertTrue(courseDisclaimerManager.isAccessGranted(repositoryEntry, id1)); - Assert.assertFalse(courseDisclaimerManager.isAccessGranted(repositoryEntry, id2)); + Assert.assertTrue(courseDisclaimerManager.isAccessGranted(repositoryEntry, id1, roles1)); + Assert.assertFalse(courseDisclaimerManager.isAccessGranted(repositoryEntry, id2, roles2)); assertThat(courseDisclaimerManager.getConsents(repositoryEntry)).hasSize(2); } @@ -170,28 +178,28 @@ public class CourseDisclaimerManagerTest extends OlatTestCase { public void revokeAllConsents() { initDisclaimer(); - courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id1, true, true); - courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id2, true, true); + courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id1, roles1, true, true); + courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id2, roles2, true, true); dbInstance.commitAndCloseSession(); - Assert.assertTrue(courseDisclaimerManager.isAccessGranted(repositoryEntry, id1)); - Assert.assertTrue(courseDisclaimerManager.isAccessGranted(repositoryEntry, id2)); + Assert.assertTrue(courseDisclaimerManager.isAccessGranted(repositoryEntry, id1, roles1)); + Assert.assertTrue(courseDisclaimerManager.isAccessGranted(repositoryEntry, id2, roles2)); courseDisclaimerManager.revokeAllConsents(repositoryEntry); dbInstance.commitAndCloseSession(); - Assert.assertFalse(courseDisclaimerManager.isAccessGranted(repositoryEntry, id1)); - Assert.assertFalse(courseDisclaimerManager.isAccessGranted(repositoryEntry, id2)); + Assert.assertFalse(courseDisclaimerManager.isAccessGranted(repositoryEntry, id1, roles1)); + Assert.assertFalse(courseDisclaimerManager.isAccessGranted(repositoryEntry, id2, roles2)); } @Test public void removeConsents() { initDisclaimer(); - courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id1, true, true); - courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id2, true, true); + courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id1, roles1, true, true); + courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id2, roles2, true, true); dbInstance.commitAndCloseSession(); @@ -210,13 +218,13 @@ public class CourseDisclaimerManagerTest extends OlatTestCase { public void revokeConsents() { initDisclaimer(); - courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id1, true, true); - courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id2, true, true); + courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id1, roles1, true, true); + courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id2, roles2, true, true); dbInstance.commitAndCloseSession(); - Assert.assertTrue(courseDisclaimerManager.isAccessGranted(repositoryEntry, id1)); - Assert.assertTrue(courseDisclaimerManager.isAccessGranted(repositoryEntry, id2)); + Assert.assertTrue(courseDisclaimerManager.isAccessGranted(repositoryEntry, id1, roles1)); + Assert.assertTrue(courseDisclaimerManager.isAccessGranted(repositoryEntry, id2, roles2)); List<Long> identitiesToRevoke = new ArrayList<>(); identitiesToRevoke.add(id1.getKey()); @@ -224,8 +232,8 @@ public class CourseDisclaimerManagerTest extends OlatTestCase { dbInstance.commitAndCloseSession(); - Assert.assertFalse(courseDisclaimerManager.isAccessGranted(repositoryEntry, id1)); - Assert.assertTrue(courseDisclaimerManager.isAccessGranted(repositoryEntry, id2)); + Assert.assertFalse(courseDisclaimerManager.isAccessGranted(repositoryEntry, id1, roles1)); + Assert.assertTrue(courseDisclaimerManager.isAccessGranted(repositoryEntry, id2, roles2)); assertThat(courseDisclaimerManager.getConsents(repositoryEntry)).hasSize(2); } @@ -235,8 +243,8 @@ public class CourseDisclaimerManagerTest extends OlatTestCase { Assert.assertFalse(courseDisclaimerManager.hasAnyEntry(repositoryEntry)); - courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id1, true, true); - courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id2, true, true); + courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id1, roles1, true, true); + courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id2, roles2, true, true); dbInstance.commitAndCloseSession(); @@ -250,8 +258,8 @@ public class CourseDisclaimerManagerTest extends OlatTestCase { Assert.assertFalse(courseDisclaimerManager.hasAnyConsent(repositoryEntry)); - courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id1, true, true); - courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id2, true, true); + courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id1, roles1, true, true); + courseDisclaimerManager.acceptDisclaimer(repositoryEntry, id2, roles2, true, true); dbInstance.commitAndCloseSession(); diff --git a/src/test/java/org/olat/user/manager/lifecycle/UserLifecycleManagerTest.java b/src/test/java/org/olat/user/manager/lifecycle/UserLifecycleManagerTest.java index f8766c3b0d0..fab1608f7dd 100644 --- a/src/test/java/org/olat/user/manager/lifecycle/UserLifecycleManagerTest.java +++ b/src/test/java/org/olat/user/manager/lifecycle/UserLifecycleManagerTest.java @@ -412,6 +412,7 @@ public class UserLifecycleManagerTest extends OlatTestCase { user.setProperty(UserConstants.INSTITUTIONALNAME, "Del-23"); user.setProperty(UserConstants.INSTITUTIONALUSERIDENTIFIER, "Del-24"); Identity identity = securityManager.createAndPersistIdentityAndUser(null, username, null, user, BaseSecurityModule.getDefaultAuthProviderIdentifier(), username, "secret"); + Roles roles = securityManager.getRoles(identity); dbInstance.commitAndCloseSession(); // add some stuff @@ -431,9 +432,9 @@ public class UserLifecycleManagerTest extends OlatTestCase { CourseFactory.saveCourse(courseOres.getResourceableId()); CourseFactory.closeCourseEditSession(courseOres.getResourceableId(), true); //a consent to the disclaimer - courseDisclaimerManager.acceptDisclaimer(course, identity, true, true); + courseDisclaimerManager.acceptDisclaimer(course, identity, roles, true, true); - Assert.assertTrue(courseDisclaimerManager.isAccessGranted(course, identity)); + Assert.assertTrue(courseDisclaimerManager.isAccessGranted(course, identity, roles)); Assert.assertEquals(identity.getName(), course.getInitialAuthor()); Assert.assertTrue(repositoryService.hasRoleExpanded(identity, GroupRoles.owner.name())); assertThat(courseDisclaimerManager.getConsents(course)).hasSize(1); -- GitLab