Skip to content

Commit

Permalink
Fix code duplication
Browse files Browse the repository at this point in the history
Moves methods for escaping out of services
  • Loading branch information
domhanak authored and paulovmr committed Jul 12, 2023
1 parent 04536b3 commit 0872802
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 46 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package org.guvnor.structure.backend;

import java.util.ArrayList;
import java.util.Collection;

import org.apache.commons.text.StringEscapeUtils;
import org.uberfire.security.Contributor;

public class InputEscapeUtils {

public static Collection<Contributor> escapeContributorsNames(Collection<Contributor> contributors) {
Collection<Contributor> escapedContributors = new ArrayList<>();
contributors.forEach((contributor -> {
String escapedName = escapeHtmlInput(contributor.getUsername());
escapedContributors.add(new Contributor(escapedName, contributor.getType()));
}));
return escapedContributors;
}

public static String escapeHtmlInput(String input) {
if (input != null) {
String escapedInput = StringEscapeUtils.escapeHtml4(input);
escapedInput = escapedInput.replace("'", "");
return escapedInput;
} else {
return null;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@
import org.uberfire.spaces.Space;
import org.uberfire.spaces.SpacesAPI;

import static org.guvnor.structure.backend.InputEscapeUtils.escapeContributorsNames;

@Service
@ApplicationScoped
public class OrganizationalUnitServiceImpl implements OrganizationalUnitService {
Expand Down Expand Up @@ -620,23 +622,4 @@ private OrganizationalUnit createDeletedOrganizationalUnit(ConfigGroup configGro
defaultGroupId,
true);
}

private Collection<Contributor> escapeContributorsNames(Collection<Contributor> contributors) {
Collection<Contributor> escapedContributors = new ArrayList<>();
contributors.forEach((contributor -> {
String escapedName = escapeHtmlInput(contributor.getUsername());
escapedContributors.add(new Contributor(escapedName, contributor.getType()));
}));
return escapedContributors;
}

String escapeHtmlInput(String input) {
if (input != null) {
String escapedInput = StringEscapeUtils.escapeHtml4(input);
escapedInput = escapedInput.replace("'", "");
return escapedInput;
} else {
return null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import javax.inject.Inject;
import javax.inject.Named;

import org.apache.commons.text.StringEscapeUtils;
import org.guvnor.common.services.backend.exceptions.ExceptionUtilities;
import org.guvnor.common.services.project.events.RepositoryContributorsUpdatedEvent;
import org.guvnor.structure.backend.backcompat.BackwardCompatibleUtil;
Expand Down Expand Up @@ -68,6 +67,7 @@
import org.uberfire.spaces.Space;
import org.uberfire.spaces.SpacesAPI;

import static org.guvnor.structure.backend.InputEscapeUtils.escapeContributorsNames;
import static org.guvnor.structure.repositories.EnvironmentParameters.CRYPT_PREFIX;
import static org.guvnor.structure.repositories.EnvironmentParameters.SECURE_PREFIX;
import static org.guvnor.structure.repositories.EnvironmentParameters.SCHEME;
Expand Down Expand Up @@ -620,25 +620,6 @@ private void addConfiguration(final RepositoryConfiguration repositoryConfigurat
}
}

public Collection<Contributor> escapeContributorsNames(Collection<Contributor> contributors) {
Collection<Contributor> escapedContributors = new ArrayList<>();
contributors.forEach((contributor -> {
String escapedName = escapeHtmlInput(contributor.getUsername());
escapedContributors.add(new Contributor(escapedName, contributor.getType()));
}));
return escapedContributors;
}

String escapeHtmlInput(String input) {
if (input != null) {
String escapedInput = StringEscapeUtils.escapeHtml4(input);
escapedInput = escapedInput.replace("'", "");
return escapedInput;
} else {
return null;
}
}

public class NoActiveSpaceInTheContext extends RuntimeException {

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import org.uberfire.spaces.Space;
import org.uberfire.spaces.SpacesAPI;

import static org.guvnor.structure.backend.InputEscapeUtils.escapeHtmlInput;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
Expand Down Expand Up @@ -270,14 +271,14 @@ public void createValidOrganizationalUnitTest() {
assertEquals(contributors, ou.getContributors());
Assertions.assertThat(ou.getContributors()).hasSize(1);
Assertions.assertThat(ou.getContributors()).hasOnlyOneElementSatisfying((contributor) -> {
contributor.getUsername().equals(organizationalUnitService.escapeHtmlInput(ADMIN));
contributor.getUsername().equals(escapeHtmlInput(ADMIN));
});
}

@Test
public void createOrganizationalUnitWithPersistentXssInContributorTest() {
final String persistentXssContributor = "<img/src/onerror=alert(\"XSS\")>";
final String escapedPersistentXssContributor = organizationalUnitService.escapeHtmlInput(persistentXssContributor);
final String escapedPersistentXssContributor = escapeHtmlInput(persistentXssContributor);

List<Contributor> contributors = new ArrayList<>();
contributors.add(new Contributor(persistentXssContributor,
Expand Down Expand Up @@ -306,10 +307,10 @@ public void createOrganizationalUnitWithPersistentXssInContributorTest() {
@Test
public void createOrganizationalUnitWithPersistentXssAndValidContributorTest() {
final String persistentXssContributor = "<img/src/onerror=alert(\"XSS\")>";
final String escapedPersistentXssContributor = organizationalUnitService.escapeHtmlInput(persistentXssContributor);
final String escapedAdminContributor = organizationalUnitService.escapeHtmlInput(ADMIN);
final String escapedPersistentXssContributor = escapeHtmlInput(persistentXssContributor);
final String escapedAdminContributor = escapeHtmlInput(ADMIN);
final String regularContributor = "head_technician_junior-intern";
final String escapedRegularContributor = organizationalUnitService.escapeHtmlInput(regularContributor);
final String escapedRegularContributor = escapeHtmlInput(regularContributor);

List<Contributor> contributors = new ArrayList<>();
contributors.add(new Contributor(persistentXssContributor,
Expand Down Expand Up @@ -401,7 +402,7 @@ public void testUpdateOrganizationalUnit() {
@Test
public void testContributorsPersistentXssOnUpdateOrganizationalUnit() {
final String persistentXssContributor = "<img/src/onerror=alert(\"XSS\")>";
final String escapedPersistentXssContributor = organizationalUnitService.escapeHtmlInput(persistentXssContributor);
final String escapedPersistentXssContributor = escapeHtmlInput(persistentXssContributor);

OrganizationalUnit organizationalUnit =
organizationalUnitService.updateOrganizationalUnit(SPACE_NAME,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.uberfire.spaces.Space;
import org.uberfire.spaces.SpacesAPI;

import static org.guvnor.structure.backend.InputEscapeUtils.escapeHtmlInput;
import static org.junit.Assert.*;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
Expand Down Expand Up @@ -195,7 +196,7 @@ public void updateContributorsWithXSSNameTest() {
when(registry.getBatch(anyString())).thenReturn(new SpaceConfigStorageRegistryImpl.SpaceStorageBatchImpl(spaceConfigStorage));

final String xssName = "<img/src/onerror=alert(\"XSS\")>";
final String escapedXssName = repositoryService.escapeHtmlInput(xssName);
final String escapedXssName = escapeHtmlInput(xssName);
repositoryService.updateContributors(repository,
Collections.singletonList(new Contributor(xssName,
ContributorType.OWNER)));
Expand Down

0 comments on commit 0872802

Please sign in to comment.