diff --git a/src/main/java/org/olat/core/util/filter/impl/OWASPAntiSamyXSSFilter.java b/src/main/java/org/olat/core/util/filter/impl/OWASPAntiSamyXSSFilter.java index bea7128ddae15c69f05ae64f16cf09dbd342854d..51d85a3df85ef6e7b82846997836a6bd8c9684b8 100644 --- a/src/main/java/org/olat/core/util/filter/impl/OWASPAntiSamyXSSFilter.java +++ b/src/main/java/org/olat/core/util/filter/impl/OWASPAntiSamyXSSFilter.java @@ -19,11 +19,14 @@ */ package org.olat.core.util.filter.impl; +import java.io.IOException; import java.io.InputStream; import java.io.PrintWriter; +import java.io.StringReader; import java.io.StringWriter; import java.io.Writer; +import org.cyberneko.html.parsers.SAXParser; import org.olat.core.logging.OLATRuntimeException; import org.olat.core.logging.OLog; import org.olat.core.logging.Tracing; @@ -34,6 +37,10 @@ import org.owasp.validator.html.CleanResults; import org.owasp.validator.html.Policy; import org.owasp.validator.html.PolicyException; import org.owasp.validator.html.ScanException; +import org.xml.sax.Attributes; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; +import org.xml.sax.helpers.DefaultHandler; /** * Description:<br> @@ -137,7 +144,7 @@ public class OWASPAntiSamyXSSFilter implements Filter { ore.printStackTrace(printWriter); } - private String getCleanHTML(String original) { + private String getCleanHTML(String original) { Policy policy; if(variant == Variant.wiki) { if(entityEncodeIntlChars) { @@ -167,7 +174,10 @@ public class OWASPAntiSamyXSSFilter implements Filter { } catch (PolicyException e) { log.error("XSS Filter policy error", e); printOriginStackTrace(); - } + } catch (IllegalStateException e) { + //Bug in Batik with rgb values in percent: rgb(100%,20%,0%) + getCleanHTMLFromBatikBug(original, policy); + } String output; try { output = cr.getCleanHTML(); @@ -182,6 +192,37 @@ public class OWASPAntiSamyXSSFilter implements Filter { return output; } + private void getCleanHTMLFromBatikBug(String original, Policy policy) { + cr = null; + try { + String rgbCleanedOriginal = cleanHtml(original); + AntiSamy as = new AntiSamy(); + cr = as.scan(rgbCleanedOriginal, policy); + } catch (ScanException e) { + log.error("XSS Filter scan error", e); + printOriginStackTrace(); + } catch (PolicyException e) { + log.error("XSS Filter policy error", e); + printOriginStackTrace(); + } catch (IllegalStateException e) { + log.error("XSS Filter policy dramatic Batik error", e); + printOriginStackTrace(); + } + } + + private String cleanHtml(String original) { + try { + HTMLCleanerHandler handler = new HTMLCleanerHandler(); + SAXParser parser = new SAXParser(); + parser.setContentHandler(handler); + parser.parse(new InputSource(new StringReader(original))); + return handler.toString(); + } catch (SAXException | IOException e) { + log.error("", e); + return ""; + } + } + public int getNumOfErrors() { if (cr != null) { return cr.getNumberOfErrors(); @@ -191,7 +232,7 @@ public class OWASPAntiSamyXSSFilter implements Filter { /** * get Errors/Messages from filter. - * This have not to be "errors", its what has been filtered and gets reported. + * This have not to be "errors", its whatR has been filtered and gets reported. * @return */ public String getOrPrintErrorMessages(){ @@ -210,4 +251,63 @@ public class OWASPAntiSamyXSSFilter implements Filter { wiki } + + /** + * The handler will remove style attributes if it detects a RGB value + * to prevent: https://issues.apache.org/jira/browse/BATIK-1149<br> + * This is a bug in Batik which doesn't understand rgb values in percent. + * + * Initial date: 16 avr. 2019<br> + * @author srosse, stephane.rosse@frentix.com, http://www.frentix.com + * + */ + private static class HTMLCleanerHandler extends DefaultHandler { + + private final StringBuilder output = new StringBuilder(4096); + + @Override + public void startElement(String uri, String localName, String qName, Attributes attributes) { + output.append("<").append(localName); + int numOfAttributes = attributes.getLength(); + for(int i=0; i<numOfAttributes; i++) { + String attrName = attributes.getLocalName(i); + String attrValue = attributes.getValue(i); + if(attrValue.contains("rgb")) { + continue; + } + + output.append(' ').append(attrName).append("="); + boolean useSingle = attrValue.indexOf('"') >= 0; + if(useSingle) { + output.append('\''); + } else { + output.append('"'); + } + output.append(attrValue); + if(useSingle) { + output.append('\''); + } else { + output.append('"'); + } + } + output.append(">"); + } + + @Override + public void characters(char[] ch, int start, int length) throws SAXException { + if(output != null) { + output.append(ch, start, length); + } + } + + @Override + public void endElement(String uri, String localName, String qName) { + output.append("</").append(localName).append(">"); + } + + @Override + public String toString() { + return output.toString(); + } + } } diff --git a/src/main/java/org/olat/course/nodes/en/ENEditGroupAreaFormController.java b/src/main/java/org/olat/course/nodes/en/ENEditGroupAreaFormController.java index 0d6396eb7b9ca555f3133af1454a4e19db7824b1..6548fb11040a138f2fc0ba71143c830ec27061dc 100644 --- a/src/main/java/org/olat/course/nodes/en/ENEditGroupAreaFormController.java +++ b/src/main/java/org/olat/course/nodes/en/ENEditGroupAreaFormController.java @@ -172,8 +172,12 @@ class ENEditGroupAreaFormController extends FormBasicController implements Gener hasAreas = areaManager.countBGAreasInContext(cev.getCourseGroupManager().getCourseResource()) > 0; hasGroups = businessGroupService.countBusinessGroups(null, cev.getCourseGroupManager().getCourseEntry()) > 0; //4. multiple groups flag - int enrollCount = multipleEnrollCount.getIntValue(); - if(!allowMultipleEnroll.isSelected(0)) enrollCount=1; + int enrollCount; + if(allowMultipleEnroll.isSelected(0)) { + enrollCount = multipleEnrollCount.getIntValue(); + } else { + enrollCount = 1; + } moduleConfig.set(ENCourseNode.CONFIG_ALLOW_MULTIPLE_ENROLL_COUNT, enrollCount); // Inform all listeners about the changed condition fireEvent(ureq, NodeEditController.NODECONFIG_CHANGED_EVENT); diff --git a/src/main/java/org/olat/group/manager/BusinessGroupServiceImpl.java b/src/main/java/org/olat/group/manager/BusinessGroupServiceImpl.java index 5ae5c4a5df6862f9159279f3d67dc4f7e7c38e72..d66116f439701edfc8748453f3c4759bf842a5b8 100644 --- a/src/main/java/org/olat/group/manager/BusinessGroupServiceImpl.java +++ b/src/main/java/org/olat/group/manager/BusinessGroupServiceImpl.java @@ -41,6 +41,7 @@ import org.olat.basesecurity.Group; import org.olat.basesecurity.GroupRoles; import org.olat.basesecurity.IdentityRef; import org.olat.basesecurity.OrganisationRoles; +import org.olat.basesecurity.SearchIdentityParams; import org.olat.collaboration.CollaborationTools; import org.olat.collaboration.CollaborationToolsFactory; import org.olat.commons.info.InfoMessageFrontendManager; @@ -48,6 +49,7 @@ import org.olat.core.CoreSpringFactory; import org.olat.core.commons.persistence.DB; import org.olat.core.commons.services.notifications.NotificationsManager; import org.olat.core.id.Identity; +import org.olat.core.id.OrganisationRef; import org.olat.core.id.Roles; import org.olat.core.logging.DBRuntimeException; import org.olat.core.logging.KnownIssueException; @@ -1036,13 +1038,13 @@ public class BusinessGroupServiceImpl implements BusinessGroupService { if(group.isAllowToLeave()) { opt = new LeaveOption(); } else { - ContactList list = getAdminContactList(group); + ContactList list = getAdminContactList(identity, group); opt = new LeaveOption(false, list); } } else if(groupModule.isAllowLeavingGroupCreatedByAuthors() && groupModule.isAllowLeavingGroupCreatedByLearners()) { opt = new LeaveOption(); } else if(!groupModule.isAllowLeavingGroupCreatedByAuthors() && !groupModule.isAllowLeavingGroupCreatedByLearners()) { - ContactList list = getAdminContactList(group); + ContactList list = getAdminContactList(identity, group); opt = new LeaveOption(false, list); } else { int numOfCoaches = countMembers(group, GroupRoles.coach.name()); @@ -1053,7 +1055,7 @@ public class BusinessGroupServiceImpl implements BusinessGroupService { if(groupModule.isAllowLeavingGroupCreatedByAuthors()) { opt = new LeaveOption(); } else { - ContactList list = getAdminContactList(group); + ContactList list = getAdminContactList(identity, group); opt = new LeaveOption(false, list); } @@ -1061,7 +1063,7 @@ public class BusinessGroupServiceImpl implements BusinessGroupService { } else if(groupModule.isAllowLeavingGroupCreatedByLearners()) { opt = new LeaveOption(); } else { - ContactList list = getAdminContactList(group); + ContactList list = getAdminContactList(identity, group); opt = new LeaveOption(false, list); } } else { @@ -1070,13 +1072,13 @@ public class BusinessGroupServiceImpl implements BusinessGroupService { if(groupModule.isAllowLeavingGroupCreatedByAuthors()) { opt = new LeaveOption(); } else { - ContactList list = getAdminContactList(group); + ContactList list = getAdminContactList(identity, group); opt = new LeaveOption(false, list); } } else if(groupModule.isAllowLeavingGroupCreatedByLearners()) { opt = new LeaveOption(); } else { - ContactList list = getAdminContactList(group); + ContactList list = getAdminContactList(identity, group); opt = new LeaveOption(false, list); } } @@ -1084,7 +1086,7 @@ public class BusinessGroupServiceImpl implements BusinessGroupService { return opt; } - public ContactList getAdminContactList(BusinessGroup group) { + private ContactList getAdminContactList(Identity identity, BusinessGroup group) { ContactList list = new ContactList("Contact"); List<Identity> coaches = getMembers(group, GroupRoles.coach.name()); if(coaches.isEmpty()) { @@ -1093,10 +1095,14 @@ public class BusinessGroupServiceImpl implements BusinessGroupService { coaches.addAll(repositoryService.getMembers(entries, RepositoryEntryRelationType.all, GroupRoles.coach.name())); if(coaches.isEmpty()) { - //get system administrators - OrganisationRoles[] adminRoles = { OrganisationRoles.administrator }; - List<Identity> admins = securityManager.getIdentitiesByPowerSearch(null, null, false, adminRoles, - null, null, null, null, null, Identity.STATUS_VISIBLE_LIMIT); + //get system administrators of the user's organisations + Roles roles = securityManager.getRoles(identity); + List<OrganisationRef> identityOrgs = roles.getOrganisationsWithRole(OrganisationRoles.user); + SearchIdentityParams identityParams = new SearchIdentityParams(); + identityParams.setOrganisations(identityOrgs); + identityParams.setRoles(new OrganisationRoles[]{ OrganisationRoles.administrator }); + identityParams.setStatus(Identity.STATUS_VISIBLE_LIMIT); + List<Identity> admins = securityManager.getIdentitiesByPowerSearch(identityParams, 0, -1); list.addAllIdentites(admins); } } diff --git a/src/main/java/org/olat/ims/qti/container/ItemContext.java b/src/main/java/org/olat/ims/qti/container/ItemContext.java index e1dc343cfba563be2dcfdf6ac5028655d5ad4003..3669819745337036331a1fa3195c3767144c0cb9 100644 --- a/src/main/java/org/olat/ims/qti/container/ItemContext.java +++ b/src/main/java/org/olat/ims/qti/container/ItemContext.java @@ -210,7 +210,7 @@ public class ItemContext implements Serializable { for (Iterator<Node> responses = el_labels.iterator(); responses.hasNext();) { Element response = (Element) responses.next(); Element parent = response.getParent(); - int pos = parent.indexOf(response); + int pos = parent.elements().indexOf(response); posList[j++] = pos; respList.add((Element)response.clone()); // need to use clones so they are not // attached anymore @@ -224,9 +224,9 @@ public class ItemContext implements Serializable { Element child = respList.get(i); List<Element> elements = parent.elements(); if(pos < elements.size()) { - parent.elements().set(pos, child); + elements.set(pos, child); } else { - parent.elements().add(child); + elements.add(child); } } return shuffleItem; diff --git a/src/test/java/org/olat/core/util/filter/impl/XSSFilterTest.java b/src/test/java/org/olat/core/util/filter/impl/XSSFilterTest.java index 18d4574cf6bfd6a28979f579d47f601b34d4180d..0cea7ead23c3dcce63db1fd381250b05c8490a83 100644 --- a/src/test/java/org/olat/core/util/filter/impl/XSSFilterTest.java +++ b/src/test/java/org/olat/core/util/filter/impl/XSSFilterTest.java @@ -237,6 +237,16 @@ public class XSSFilterTest { // t("<span style=\"font-family: serif, arial;\">preformated</span>", "<span style=\"font-family: courier new , courier;\">preformated</span>"); t("<span class=\"schoen\">irgendwas</span>", "<span class=\"schoen\">irgendwas</span>"); } + + /** + * This checks a bug in Batik + */ + @Test + public void test_style_rgb(){ + t("<p style=\"background-color: rgb(0%,0,0);\">background</p>", "<p>background</p>"); + t("<p style=\"background-color: rgba(100%,0,0);\">background</p>", "<p style=\"\">background</p>"); + t("<p style=\"background-color: rgb(100,50,50);\">background</p>", "<p style=\"background-color: rgb(100,50,50);\">background</p>"); + } @Test public void test_tiny_lists(){ diff --git a/src/test/java/org/olat/group/test/BusinessGroupServiceTest.java b/src/test/java/org/olat/group/test/BusinessGroupServiceTest.java index 0e15e3437b286f76ebff19be86901e836d32c30a..0a5f25e3dc3b727066cd0ebc0590d67f9fb18113 100644 --- a/src/test/java/org/olat/group/test/BusinessGroupServiceTest.java +++ b/src/test/java/org/olat/group/test/BusinessGroupServiceTest.java @@ -42,12 +42,15 @@ import org.olat.basesecurity.BaseSecurity; import org.olat.basesecurity.Group; import org.olat.basesecurity.GroupRoles; import org.olat.basesecurity.OrganisationRoles; +import org.olat.basesecurity.OrganisationService; import org.olat.basesecurity.manager.GroupDAO; import org.olat.collaboration.CollaborationTools; import org.olat.collaboration.CollaborationToolsFactory; 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.core.id.User; import org.olat.core.logging.OLog; import org.olat.core.logging.Tracing; import org.olat.core.util.mail.ContactList; @@ -67,6 +70,7 @@ import org.olat.resource.accesscontrol.ResourceReservation; import org.olat.resource.accesscontrol.manager.ACReservationDAO; import org.olat.test.JunitTestHelper; import org.olat.test.OlatTestCase; +import org.olat.user.UserManager; import org.springframework.beans.factory.annotation.Autowired; /** @@ -86,6 +90,8 @@ public class BusinessGroupServiceTest extends OlatTestCase { @Autowired private ACService acService; @Autowired + private UserManager userManager; + @Autowired private ACReservationDAO reservationDao; @Autowired private BaseSecurity securityManager; @@ -96,6 +102,8 @@ public class BusinessGroupServiceTest extends OlatTestCase { @Autowired private BusinessGroupService businessGroupService; @Autowired + private OrganisationService organisationService; + @Autowired private MailModule mailModule; // Identities for tests @@ -1145,6 +1153,46 @@ public class BusinessGroupServiceTest extends OlatTestCase { } } + @Test + public void allowToLeavingBusinessGroup_subOrganisation() { + // a special + String uuid = UUID.randomUUID().toString(); + Organisation organisation = organisationService.getDefaultOrganisation(); + Organisation subOrganisation = organisationService + .createOrganisation("Sub-organisation", uuid, "", organisation, null); + + // create an administrator + String adminName = "admin" + uuid; + User adminUser = userManager.createUser("Admin", "Istrator", uuid + "admin@openolat.org"); + Identity adminIdentity = securityManager.createAndPersistIdentityAndUser(adminName, null, adminUser, null, adminName); + organisationService.addMember(subOrganisation, adminIdentity, OrganisationRoles.user); + organisationService.addMember(subOrganisation, adminIdentity, OrganisationRoles.administrator); + //create a user + String userName = "user" + uuid; + User user = userManager.createUser("Us", "er", uuid + "user@openolat.org"); + Identity userIdentity = securityManager.createAndPersistIdentityAndUser(userName, null, user, null, userName); + organisationService.addMember(subOrganisation, userIdentity, OrganisationRoles.user); + dbInstance.commitAndCloseSession(); + + BusinessGroup group = businessGroupService.createBusinessGroup(null, "Leaving group", "But you cannot leave :-(", new Integer(0), new Integer(2), false, false, null); + businessGroupRelationDao.addRole(userIdentity, group, GroupRoles.participant.name()); + dbInstance.commitAndCloseSession(); + + //set to not allowed to leave + group = businessGroupService.updateAllowToLeaveBusinessGroup(group, false); + dbInstance.commitAndCloseSession(); + + //check the authors group leaving option + LeaveOption optionToLeave = businessGroupService.isAllowToLeaveBusinessGroup(userIdentity, group); + Assert.assertNotNull(optionToLeave); + Assert.assertFalse(optionToLeave.isAllowToLeave()); + ContactList contacts = optionToLeave.getContacts(); + Collection<Identity> contactList = contacts.getIdentiEmails().values(); + Assert.assertNotNull(contactList); + Assert.assertFalse(contactList.isEmpty()); + Assert.assertTrue(contactList.contains(adminIdentity)); + } + @Ignore @Test public void parallelRemoveParticipants() { mailModule.setInterSystem(true);