diff --git a/openid-connect-common/pom.xml b/openid-connect-common/pom.xml index 79123a854..54cc0ef55 100644 --- a/openid-connect-common/pom.xml +++ b/openid-connect-common/pom.xml @@ -94,7 +94,7 @@ <dependency> <groupId>org.codehaus.groovy</groupId> - <artifactId>groovy-all</artifactId> + <artifactId>groovy</artifactId> </dependency> <dependency> <groupId>org.spockframework</groupId> @@ -117,7 +117,7 @@ <plugin> <groupId>org.codehaus.gmavenplus</groupId> <artifactId>gmavenplus-plugin</artifactId> - <version>1.7.0</version> + <version>1.8.1</version> <executions> <execution> <goals> @@ -135,7 +135,7 @@ <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-surefire-plugin</artifactId> - <version>2.18.1</version> + <version>2.22.2</version> <configuration> <excludedGroups combine.self="override"/> <testClassesDirectory>${project.build.testOutputDirectory}</testClassesDirectory> diff --git a/openid-connect-common/src/main/java/org/mitre/openid/connect/web/UserInfoInterceptor.java b/openid-connect-common/src/main/java/org/mitre/openid/connect/web/UserInfoInterceptor.java index 43d9c8591..9fcfe795d 100644 --- a/openid-connect-common/src/main/java/org/mitre/openid/connect/web/UserInfoInterceptor.java +++ b/openid-connect-common/src/main/java/org/mitre/openid/connect/web/UserInfoInterceptor.java @@ -83,7 +83,7 @@ public class UserInfoInterceptor extends HandlerInterceptorAdapter { OIDCAuthenticationToken oidc = (OIDCAuthenticationToken) auth; UserInfo userInfo = oidc.getUserInfo(); if (userInfo != null) { - santiseUserInfo(userInfo); + sanitiseUserInfo(userInfo); request.setAttribute("userInfo", userInfo); request.setAttribute("userInfoJson", userInfo.toJson()); } else { @@ -99,7 +99,7 @@ public class UserInfoInterceptor extends HandlerInterceptorAdapter { // if we have one, inject it so views can use it if (user != null) { - santiseUserInfo(user); + sanitiseUserInfo(user); request.setAttribute("userInfo", user); request.setAttribute("userInfoJson", user.toJson()); } @@ -110,39 +110,38 @@ public class UserInfoInterceptor extends HandlerInterceptorAdapter { return true; } - private UserInfo santiseUserInfo(final UserInfo userInfo) { - userInfo.setSub(nullCheckClean(userInfo.getSub())); - userInfo.setPreferredUsername(nullCheckClean(userInfo.getPreferredUsername())); - userInfo.setName(nullCheckClean(userInfo.getName())); - userInfo.setGivenName(nullCheckClean(userInfo.getGivenName())); - userInfo.setFamilyName(nullCheckClean(userInfo.getFamilyName())); - userInfo.setMiddleName(nullCheckClean(userInfo.getMiddleName())); - userInfo.setNickname(nullCheckClean(userInfo.getNickname())); - userInfo.setProfile(nullCheckClean(userInfo.getProfile())); - userInfo.setPicture(nullCheckClean(userInfo.getPicture())); - userInfo.setWebsite(nullCheckClean(userInfo.getWebsite())); - userInfo.setEmail(nullCheckClean(userInfo.getEmail())); - userInfo.setGender(nullCheckClean(userInfo.getGender())); - userInfo.setLocale(nullCheckClean(userInfo.getLocale())); - userInfo.setPhoneNumber(nullCheckClean(userInfo.getPhoneNumber())); - userInfo.setUpdatedTime(nullCheckClean(userInfo.getUpdatedTime())); - userInfo.setBirthdate(nullCheckClean(userInfo.getBirthdate())); + private void sanitiseUserInfo(final UserInfo userInfo) { + userInfo.setSub(sanitise(userInfo.getSub())); + userInfo.setPreferredUsername(sanitise(userInfo.getPreferredUsername())); + userInfo.setName(sanitise(userInfo.getName())); + userInfo.setGivenName(sanitise(userInfo.getGivenName())); + userInfo.setFamilyName(sanitise(userInfo.getFamilyName())); + userInfo.setMiddleName(sanitise(userInfo.getMiddleName())); + userInfo.setNickname(sanitise(userInfo.getNickname())); + userInfo.setProfile(sanitise(userInfo.getProfile())); + userInfo.setPicture(sanitise(userInfo.getPicture())); + userInfo.setWebsite(sanitise(userInfo.getWebsite())); + userInfo.setEmail(sanitise(userInfo.getEmail())); + userInfo.setGender(sanitise(userInfo.getGender())); + userInfo.setLocale(sanitise(userInfo.getLocale())); + userInfo.setPhoneNumber(sanitise(userInfo.getPhoneNumber())); + userInfo.setUpdatedTime(sanitise(userInfo.getUpdatedTime())); + userInfo.setBirthdate(sanitise(userInfo.getBirthdate())); Address userInfoAddress = userInfo.getAddress(); if (userInfoAddress != null) { - userInfoAddress.setFormatted(nullCheckClean(userInfoAddress.getFormatted())); - userInfoAddress.setStreetAddress(nullCheckClean(userInfoAddress.getStreetAddress())); - userInfoAddress.setLocality(nullCheckClean(userInfoAddress.getLocality())); - userInfoAddress.setRegion(nullCheckClean(userInfoAddress.getRegion())); - userInfoAddress.setPostalCode(nullCheckClean(userInfoAddress.getPostalCode())); - userInfoAddress.setCountry(nullCheckClean(userInfoAddress.getCountry())); + userInfoAddress.setFormatted(sanitise(userInfoAddress.getFormatted())); + userInfoAddress.setStreetAddress(sanitise(userInfoAddress.getStreetAddress())); + userInfoAddress.setLocality(sanitise(userInfoAddress.getLocality())); + userInfoAddress.setRegion(sanitise(userInfoAddress.getRegion())); + userInfoAddress.setPostalCode(sanitise(userInfoAddress.getPostalCode())); + userInfoAddress.setCountry(sanitise(userInfoAddress.getCountry())); userInfo.setAddress(userInfoAddress); } - return userInfo; } - private String nullCheckClean(String elementToClean) { + private String sanitise(String elementToClean) { final Whitelist whitelist = Whitelist.relaxed() .removeTags("a") .removeProtocols("img", "src", "http", "https"); diff --git a/openid-connect-common/src/test/groovy/org/mitre/openid/connect/web/UserInfoInterceptorSpec.groovy b/openid-connect-common/src/test/groovy/org/mitre/openid/connect/web/UserInfoInterceptorSpec.groovy index 9bea3e7c0..5dd47daa6 100644 --- a/openid-connect-common/src/test/groovy/org/mitre/openid/connect/web/UserInfoInterceptorSpec.groovy +++ b/openid-connect-common/src/test/groovy/org/mitre/openid/connect/web/UserInfoInterceptorSpec.groovy @@ -23,19 +23,19 @@ class UserInfoInterceptorSpec extends Specification { when: 'The user info object is passed through the sanitise method' - UserInfo sanitisedUserInfo = userInfoInterceptor.santiseUserInfo(userInfo) + userInfoInterceptor.sanitiseUserInfo(userInfo) then: 'The malicious names have been sanitised' - sanitisedUserInfo.getName() == 'Test Test' - sanitisedUserInfo.getGivenName() == 'Test' + userInfo.getName() == 'Test Test' + userInfo.getGivenName() == 'Test' and: 'The non malicious elements have been unaffected' - sanitisedUserInfo.getSub() == '12318767' - sanitisedUserInfo.getPreferredUsername() == 'Test' - sanitisedUserInfo.getFamilyName() == 'Test' - sanitisedUserInfo.getEmail() == 'test@test.com' + userInfo.getSub() == '12318767' + userInfo.getPreferredUsername() == 'Test' + userInfo.getFamilyName() == 'Test' + userInfo.getEmail() == 'test@test.com' } } diff --git a/pom.xml b/pom.xml index a11af7e5c..98af967d3 100644 --- a/pom.xml +++ b/pom.xml @@ -522,19 +522,19 @@ </dependency> <dependency> <groupId>org.codehaus.groovy</groupId> - <artifactId>groovy-all</artifactId> - <version>2.4.13</version> + <artifactId>groovy</artifactId> + <version>2.5.9</version> <scope>test</scope> </dependency> <dependency> <groupId>org.spockframework</groupId> <artifactId>spock-core</artifactId> - <version>1.1-groovy-2.4</version> + <version>1.3-groovy-2.5</version> <scope>test</scope> <exclusions> <exclusion> <groupId>org.codehaus.groovy</groupId> - <artifactId>groovy-all</artifactId> + <artifactId>*</artifactId> </exclusion> </exclusions> </dependency>