diff --git a/.travis.yml b/.travis.yml index dff5f3a5d..53ebd0ee7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1 +1,6 @@ language: java + +install: mvn -U install --quiet -DskipTests=true -P bootstrap +script: mvn clean test -P bootstrap + + diff --git a/spring-security-oauth/src/main/java/org/springframework/security/oauth/consumer/client/CoreOAuthConsumerSupport.java b/spring-security-oauth/src/main/java/org/springframework/security/oauth/consumer/client/CoreOAuthConsumerSupport.java index 3852a8203..36ab3b822 100644 --- a/spring-security-oauth/src/main/java/org/springframework/security/oauth/consumer/client/CoreOAuthConsumerSupport.java +++ b/spring-security-oauth/src/main/java/org/springframework/security/oauth/consumer/client/CoreOAuthConsumerSupport.java @@ -647,16 +647,17 @@ protected String getSignatureBaseString(Map> oauthPara Iterator>> sortedIt = sortedParameters.entrySet().iterator(); while (sortedIt.hasNext()) { Map.Entry> sortedParameter = sortedIt.next(); - for (String parameterValue : sortedParameter.getValue()) { - if (parameterValue == null) { + for (Iterator sortedParametersIterator = sortedParameter.getValue().iterator(); sortedParametersIterator.hasNext();) { + String parameterValue = sortedParametersIterator.next(); + if (parameterValue == null) { parameterValue = ""; } queryString.append(sortedParameter.getKey()).append('=').append(parameterValue); - if (sortedIt.hasNext()) { + if (sortedIt.hasNext() || sortedParametersIterator.hasNext()) { queryString.append('&'); } - } + } } StringBuilder url = new StringBuilder(requestURL.getProtocol().toLowerCase()).append("://").append(requestURL.getHost().toLowerCase()); diff --git a/spring-security-oauth/src/test/java/org/springframework/security/oauth/consumer/client/TestCoreOAuthConsumerSupport.java b/spring-security-oauth/src/test/java/org/springframework/security/oauth/consumer/client/TestCoreOAuthConsumerSupport.java index 8a2fddd6e..05e898f6f 100644 --- a/spring-security-oauth/src/test/java/org/springframework/security/oauth/consumer/client/TestCoreOAuthConsumerSupport.java +++ b/spring-security-oauth/src/test/java/org/springframework/security/oauth/consumer/client/TestCoreOAuthConsumerSupport.java @@ -38,8 +38,10 @@ import java.net.URL; import java.net.URLConnection; import java.net.URLEncoder; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; import java.util.TreeMap; @@ -79,7 +81,8 @@ public void testAfterPropertiesSet() throws Exception { try { new CoreOAuthConsumerSupport().afterPropertiesSet(); fail("should required a protected resource details service."); - } catch (IllegalArgumentException e) { + } + catch (IllegalArgumentException e) { } } @@ -146,7 +149,8 @@ public URL configureURLForProtectedAccess(URL url, OAuthConsumerToken accessToke try { return new URL(url.getProtocol(), url.getHost(), url.getPort(), url.getFile(), new StreamHandlerForTestingPurposes(connectionMock)); - } catch (MalformedURLException e) { + } + catch (MalformedURLException e) { throw new RuntimeException(e); } } @@ -165,7 +169,8 @@ public String getOAuthQueryString(ProtectedResourceDetails details, OAuthConsume try { support.readResource(details, url, "POST", token, null, null); fail("shouldn't have been a valid response code."); - } catch (OAuthRequestFailedException e) { + } + catch (OAuthRequestFailedException e) { // fall through... } assertFalse(connectionProps.doOutput); @@ -181,7 +186,8 @@ public String getOAuthQueryString(ProtectedResourceDetails details, OAuthConsume try { support.readResource(details, url, "POST", token, null, null); fail("shouldn't have been a valid response code."); - } catch (OAuthRequestFailedException e) { + } + catch (OAuthRequestFailedException e) { // fall through... } assertFalse(connectionProps.doOutput); @@ -198,7 +204,8 @@ public String getOAuthQueryString(ProtectedResourceDetails details, OAuthConsume try { support.readResource(details, url, "POST", token, null, null); fail("shouldn't have been a valid response code."); - } catch (InvalidOAuthRealmException e) { + } + catch (InvalidOAuthRealmException e) { // fall through... } assertFalse(connectionProps.doOutput); @@ -400,8 +407,8 @@ protected String getSignatureBaseString(Map> oauthPara when(details.getSignatureMethod()).thenReturn(HMAC_SHA1SignatureMethod.SIGNATURE_NAME); SharedConsumerSecret secret = new SharedConsumerSecretImpl("shh!!!"); when(details.getSharedSecret()).thenReturn(secret); - when(sigFactory.getSignatureMethod(HMAC_SHA1SignatureMethod.SIGNATURE_NAME, secret, null)).thenReturn( - sigMethod); + when(sigFactory.getSignatureMethod(HMAC_SHA1SignatureMethod.SIGNATURE_NAME, secret, null)) + .thenReturn(sigMethod); when(sigMethod.sign("MYSIGBASESTRING")).thenReturn("MYSIGNATURE"); Map> params = support.loadOAuthParameters(details, url, token, "POST", null); @@ -446,6 +453,33 @@ public void testGetSignatureBaseString() throws Exception { baseString); } + @Test + public void testGetSignatureBaseStringSimple() throws Exception { + Map> oauthParams = new HashMap>(); + oauthParams.put("foo", Collections.singleton((CharSequence) "bar")); + oauthParams.put("bar", new LinkedHashSet(Arrays. asList("120", "24"))); + + CoreOAuthConsumerSupport support = new CoreOAuthConsumerSupport(); + + String baseString = support.getSignatureBaseString(oauthParams, new URL("http://photos.example.net/photos"), + "get"); + assertEquals("GET&http%3A%2F%2Fphotos.example.net%2Fphotos&bar%3D120%26bar%3D24%26foo%3Dbar", baseString); + } + + // See SECOAUTH-383 + @Test + public void testGetSignatureBaseStringMultivaluedLast() throws Exception { + Map> oauthParams = new HashMap>(); + oauthParams.put("foo", Collections.singleton((CharSequence) "bar")); + oauthParams.put("pin", new LinkedHashSet(Arrays. asList("2", "1"))); + + CoreOAuthConsumerSupport support = new CoreOAuthConsumerSupport(); + + String baseString = support.getSignatureBaseString(oauthParams, new URL("http://photos.example.net/photos"), + "get"); + assertEquals("GET&http%3A%2F%2Fphotos.example.net%2Fphotos&foo%3Dbar%26pin%3D1%26pin%3D2", baseString); + } + static class StreamHandlerForTestingPurposes extends Handler { private final HttpURLConnectionForTestingPurposes connection; @@ -469,7 +503,7 @@ static class HttpURLConnectionForTestingPurposes extends HttpURLConnection { /** * Constructor for the HttpURLConnection. - * + * * @param u the URL */ public HttpURLConnectionForTestingPurposes(URL u) { @@ -490,11 +524,17 @@ public void connect() throws IOException { static class ConnectionProps { public int responseCode; + public String responseMessage; + public String method; + public Boolean doOutput; + public Boolean connected; + public OutputStream outputStream; + public final Map headerFields = new TreeMap(); public void reset() {