Skip to content

Commit

Permalink
Decouple Apache HTTP from SignatureAuthMethod
Browse files Browse the repository at this point in the history
  • Loading branch information
@SMadani
SMadani committed May 21, 2024
1 parent 129346c commit 9ce9554
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 210 deletions.
15 changes: 13 additions & 2 deletions src/main/java/com/vonage/client/AbstractMethod.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,17 @@
import com.vonage.client.auth.*;
import org.apache.commons.logging.LogFactory;
import org.apache.http.HttpResponse;
import org.apache.http.NameValuePair;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.client.methods.RequestBuilder;
import org.apache.http.impl.client.BasicResponseHandler;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
import java.util.stream.Collectors;

/**
Expand Down Expand Up @@ -110,9 +113,17 @@ public ResultT execute(RequestT request) throws VonageResponseParseException, Vo
*/
protected RequestBuilder applyAuth(RequestBuilder request) throws VonageClientException {
AuthMethod am = getAuthMethod();

if (am instanceof SignatureAuthMethod) {
// TODO sort out this complete mess of an AuthMethod
return ((SignatureAuthMethod) am).apply(request);
Map<String, String> params = request.getParameters().stream().collect(Collectors.toMap(
NameValuePair::getName,
NameValuePair::getValue,
(v1, v2) -> v1,
TreeMap::new
));
((SignatureAuthMethod) am).apply(params);
request.getParameters().clear();
params.forEach(request::addParameter);
}
else if (am instanceof HeaderAuthMethod) {
return request.setHeader("Authorization", ((HeaderAuthMethod) am).getHeaderValue());
Expand Down
75 changes: 34 additions & 41 deletions src/main/java/com/vonage/client/auth/RequestSigning.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.http.NameValuePair;
import org.apache.http.message.BasicNameValuePair;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
Expand All @@ -31,6 +30,7 @@
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import java.util.stream.Collectors;

/**
* A helper class for generating or verifying MD5 signatures when signing REST requests for submission to Vonage.
Expand All @@ -44,6 +44,10 @@ public class RequestSigning {

private static final Log log = LogFactory.getLog(RequestSigning.class);

private static String clean(String str) {
return str == null ? null : str.replaceAll("[=&]", "_");
}

/**
* Signs a set of request parameters.
* <p>
Expand All @@ -54,9 +58,11 @@ public class RequestSigning {
* @param params List of NameValuePair instances containing the query parameters for the request that is to be signed
* @param secretKey the pre-shared secret key held by the client
*
* @deprecated Use {@link #constructSignatureForRequestParameters(Map, String, HashUtil.HashType)}.
*/
@Deprecated
public static void constructSignatureForRequestParameters(List<NameValuePair> params, String secretKey) {
constructSignatureForRequestParameters(params, secretKey, Instant.now().getEpochSecond());
constructSignatureForRequestParameters(params, secretKey, HashUtil.HashType.HMAC_MD5);
}

/**
Expand All @@ -66,30 +72,33 @@ public static void constructSignatureForRequestParameters(List<NameValuePair> pa
* Uses the supplied pre-shared secret key to generate the signature.
*
* @param params List of NameValuePair instances containing the query parameters for the request that is to be signed
* @param secretKey the pre-shared secret key held by the client
* @param hashType The type of hash that is to be used in construction
* @param secretKey the pre-shared secret key held by the client.
* @param hashType The type of hash that is to be used in construction.
* @deprecated Use {@link #constructSignatureForRequestParameters(Map, String, HashUtil.HashType)}.
*/
@Deprecated
public static void constructSignatureForRequestParameters(List<NameValuePair> params, String secretKey, HashUtil.HashType hashType) {
constructSignatureForRequestParameters(params, secretKey, Instant.now().getEpochSecond(), hashType);
Map<String, String> sortedParams = params.stream().collect(Collectors.toMap(
NameValuePair::getName,
NameValuePair::getValue,
(v1, v2) -> v1,
TreeMap::new
));
constructSignatureForRequestParameters(sortedParams, secretKey, hashType);
}

/**
* Signs a set of request parameters.
* <p>
* Generates additional parameters to represent the timestamp and generated signature.
* Uses the supplied pre-shared secret key to generate the signature.
* Hashing strategy is MD5.
*
* @param params List of NameValuePair instances containing the query parameters for the request that is to be signed
* @param secretKey the pre-shared secret key held by the client
* @param currentTimeSeconds the current time in seconds since 1970-01-01
*
* @param hashType The type of hash that is to be used in construction
*/
protected static void constructSignatureForRequestParameters(List<NameValuePair> params,
String secretKey,
long currentTimeSeconds) {
// First, inject a 'timestamp=' parameter containing the current time in seconds since Jan 1st 1970
constructSignatureForRequestParameters(params, secretKey, currentTimeSeconds, HashUtil.HashType.MD5);
public static void constructSignatureForRequestParameters(Map<String, String> params, String secretKey, HashUtil.HashType hashType) {
constructSignatureForRequestParameters(params, secretKey, Instant.now().getEpochSecond(), hashType);
}

/**
Expand All @@ -98,38 +107,27 @@ protected static void constructSignatureForRequestParameters(List<NameValuePair>
* Generates additional parameters to represent the timestamp and generated signature.
* Uses the supplied pre-shared secret key to generate the signature.
*
* @param params List of NameValuePair instances containing the query parameters for the request that is to be signed.
* @param params Query parameters for the request that is to be signed.
* @param secretKey the pre-shared secret key held by the client.
* @param currentTimeSeconds the current time in seconds since 1970-01-01.
* @param hashType Hash type to be used to construct request parameters.
*
*/
protected static void constructSignatureForRequestParameters(List<NameValuePair> params,
static void constructSignatureForRequestParameters(Map<String, String> params,
String secretKey,
long currentTimeSeconds,
HashUtil.HashType hashType) {
// First, inject a 'timestamp=' parameter containing the current time in seconds since Jan 1st 1970
params.add(new BasicNameValuePair(PARAM_TIMESTAMP, Long.toString(currentTimeSeconds)));

Map<String, String> sortedParams = new TreeMap<>();
for (NameValuePair param: params) {
String name = param.getName();
String value = param.getValue();
if (name.equals(PARAM_SIGNATURE)) {
continue;
}
if (value == null) {
value = "";
}
if (!value.trim().isEmpty()) {
sortedParams.put(name, value);
}
}
params.put(PARAM_TIMESTAMP, Long.toString(currentTimeSeconds));

// Now, walk through the sorted list of parameters and construct a string
StringBuilder sb = new StringBuilder();
for (Map.Entry<String, String> param: sortedParams.entrySet()) {
sb.append("&").append(clean(param.getKey())).append("=").append(clean(param.getValue()));
for (Map.Entry<String, String> param: params.entrySet()) {
String name = param.getKey(), value = param.getValue();
if (PARAM_SIGNATURE.equals(name) || value == null || value.trim().isEmpty()) {
continue;
}
sb.append("&").append(clean(name)).append("=").append(clean(value));
}

String str = sb.toString();
Expand All @@ -143,7 +141,7 @@ protected static void constructSignatureForRequestParameters(List<NameValuePair>

log.debug("SECURITY-KEY-GENERATION -- String [ " + str + " ] Signature [ " + hashed + " ] ");

params.add(new BasicNameValuePair(PARAM_SIGNATURE, hashed));
params.put(PARAM_SIGNATURE, hashed);
}

/**
Expand Down Expand Up @@ -198,7 +196,7 @@ protected static boolean verifyRequestSignature(String contentType,
*
* @return true if the signature is correct for this request and secret key.
*/
protected static boolean verifyRequestSignature(String contentType,
static boolean verifyRequestSignature(String contentType,
InputStream inputStream,
Map<String, String[]> parameterMap,
String secretKey,
Expand Down Expand Up @@ -258,7 +256,7 @@ protected static boolean verifyRequestSignature(String contentType,
}


// walk this sorted list of parameters and construct a string
// Walk this sorted list of parameters and construct a string
StringBuilder sb = new StringBuilder();
for (Map.Entry<String, String> param : sortedParams.entrySet()) {
if (param.getKey().equals(PARAM_SIGNATURE)) continue;
Expand Down Expand Up @@ -286,9 +284,4 @@ protected static boolean verifyRequestSignature(String contentType,
suppliedSignature.toLowerCase().getBytes(StandardCharsets.UTF_8)
);
}

public static String clean(String str) {
return str == null ? null : str.replaceAll("[=&]", "_");
}

}
12 changes: 3 additions & 9 deletions src/main/java/com/vonage/client/auth/SignatureAuthMethod.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@
package com.vonage.client.auth;

import com.vonage.client.auth.hashutils.HashUtil;
import org.apache.http.NameValuePair;
import org.apache.http.client.methods.RequestBuilder;
import java.util.List;
import java.util.Map;

public class SignatureAuthMethod extends AuthMethod {
private static final int SORT_KEY = 20;
Expand All @@ -45,12 +43,8 @@ public int getSortKey() {
return SORT_KEY;
}

public RequestBuilder apply(RequestBuilder request) {
request.addParameter("api_key", apiKey);
List<NameValuePair> params = request.getParameters();
public void apply(Map<String, String> params) {
params.put("api_key", apiKey);
RequestSigning.constructSignatureForRequestParameters(params, apiSecret, hashType);
int last = params.size() - 1;
request.addParameters(params.get(last), params.get(last - 1));
return request;
}
}
97 changes: 26 additions & 71 deletions src/test/java/com/vonage/client/VonageClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,22 @@
import com.vonage.client.voice.CallStatus;
import io.jsonwebtoken.Claims;
import io.jsonwebtoken.Jwts;
import org.apache.http.NameValuePair;
import org.apache.http.client.methods.RequestBuilder;
import org.apache.http.message.BasicNameValuePair;
import static org.junit.jupiter.api.Assertions.*;
import org.junit.jupiter.api.*;
import java.nio.file.Paths;
import java.security.KeyFactory;
import java.security.PublicKey;
import java.security.spec.X509EncodedKeySpec;
import java.util.List;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.UUID;

public class VonageClientTest extends ClientTest<VonageClient> {
private static final UUID APPLICATION_ID = UUID.randomUUID();
private final TestUtils testUtils = new TestUtils();

final String key = "api-key", secret = "api-secret";

@Test
public void testConstructVonageClient() throws Exception {
byte[] keyBytes = testUtils.loadKey("test/keys/application_key");
Expand Down Expand Up @@ -99,21 +99,21 @@ public void testGenerateJwt() throws Exception {
@Test
public void testSoloApiKeyThrowsException() {
assertThrows(VonageClientCreationException.class, () ->
VonageClient.builder().apiKey("api-key").build()
VonageClient.builder().apiKey(key).build()
);
}

@Test
public void testSoloApiSecret() {
assertThrows(VonageClientCreationException.class, () ->
VonageClient.builder().apiSecret("api-secret").build()
VonageClient.builder().apiSecret(secret).build()
);
}

@Test
public void testSoloSignatureSecret() {
assertThrows(VonageClientCreationException.class, () ->
VonageClient.builder().signatureSecret("api-secret").build()
VonageClient.builder().signatureSecret(secret).build()
);
}

Expand All @@ -140,77 +140,37 @@ public void testApiKeyWithSecret() throws VonageUnacceptableAuthException {

@Test
public void testApiKeyWithSignatureSecret() throws Exception {
VonageClient vonageClient = VonageClient.builder().apiKey("api-key").signatureSecret("api-secret").build();
AuthCollection authCollection = vonageClient.getHttpWrapper().getAuthCollection();
var vonageClient = VonageClient.builder()
.apiKey(key).signatureSecret(secret).build();

RequestBuilder requestBuilder = RequestBuilder.get();
authCollection.getAuth(SignatureAuthMethod.class).apply(requestBuilder);
var authCollection = vonageClient.getHttpWrapper().getAuthCollection();

List<NameValuePair> parameters = requestBuilder.getParameters();
Map<String, String> params = new LinkedHashMap<>();
authCollection.getAuth(SignatureAuthMethod.class).apply(params);

// This is messy but trying to generate a signature auth method and then comparing with what's on the request
// could have a race condition depending on the returned timestamp.

// So, we're going to generate the signature after trying to determine what the timestamp is.
String timestamp = parameters
.stream()
.filter(pair -> "timestamp".equals(pair.getName()))
.findFirst()
.orElse(new BasicNameValuePair("", ""))
.getValue();

String sig = HashUtil.calculate("&api_key=api-key&timestamp=" + timestamp + "api-secret", HashUtil.HashType.MD5);
assertContainsParam(parameters, "sig", sig);
}
String timestamp = params.get("timestamp");
String sig = HashUtil.calculate("&api_key="+key+"&timestamp=" + timestamp + secret, HashUtil.HashType.MD5);
assertEquals(sig, params.get("sig"));

@Test
public void testApiKeyWithSignatureSecretAsHMACSHA256() throws Exception {
VonageClient vonageClient = VonageClient.builder().hashType(HashUtil.HashType.HMAC_SHA256).apiKey("api-key").signatureSecret("api-secret").build();
AuthCollection authCollection = vonageClient.getHttpWrapper().getAuthCollection();
for (var hashType : HashUtil.HashType.values()) {
if (hashType == HashUtil.HashType.MD5) continue;

RequestBuilder requestBuilder = RequestBuilder.get();
authCollection.getAuth(SignatureAuthMethod.class).apply(requestBuilder);
vonageClient = VonageClient.builder()
.apiKey(key).signatureSecret(secret).hashType(hashType).build();

List<NameValuePair> parameters = requestBuilder.getParameters();
authCollection = vonageClient.getHttpWrapper().getAuthCollection();

// This is messy but trying to generate a signature auth method and then comparing with what's on the request
// could have a race condition depending on the returned timestamp.
params = new LinkedHashMap<>();
authCollection.getAuth(SignatureAuthMethod.class).apply(params);

// So, we're going to generate the signature after trying to determine what the timestamp is.
String timestamp = parameters
.stream()
.filter(pair -> "timestamp".equals(pair.getName()))
.findFirst()
.orElse(new BasicNameValuePair("", ""))
.getValue();

String sig = HashUtil.calculate("&api_key=api-key&timestamp=" + timestamp, "api-secret", "UTF-8", HashUtil.HashType.HMAC_SHA256);
assertContainsParam(parameters, "sig", sig);
}

@Test
public void testApiKeyWithSignatureSecretAsHmacSHA256() throws Exception {
VonageClient vonageClient = VonageClient.builder().hashType(HashUtil.HashType.HMAC_SHA256).apiKey("api-key").signatureSecret("api-secret").build();
AuthCollection authCollection = vonageClient.getHttpWrapper().getAuthCollection();

RequestBuilder requestBuilder = RequestBuilder.get();
authCollection.getAuth(SignatureAuthMethod.class).apply(requestBuilder);

List<NameValuePair> parameters = requestBuilder.getParameters();

// This is messy but trying to generate a signature auth method and then comparing with what's on the request
// could have a race condition depending on the returned timestamp.

// So, we're going to generate the signature after trying to determine what the timestamp is.
String timestamp = parameters
.stream()
.filter(pair -> "timestamp".equals(pair.getName()))
.findFirst()
.orElse(new BasicNameValuePair("", ""))
.getValue();

String sig = HashUtil.calculate("&api_key=api-key&timestamp=" + timestamp, "api-secret", "UTF-8", HashUtil.HashType.HMAC_SHA256);
assertContainsParam(parameters, "sig", sig);
timestamp = params.get("timestamp");
sig = HashUtil.calculate("&api_key="+key+"&timestamp=" + timestamp, secret, "UTF-8", hashType);
assertEquals(sig, params.get("sig"));
}
}

@Test
Expand Down Expand Up @@ -303,9 +263,4 @@ public void testSubClientGetters() {
assertNotNull(client.getVerify2Client());
assertNotNull(client.getVideoClient());
}

private void assertContainsParam(List<NameValuePair> params, String key, String value) {
NameValuePair item = new BasicNameValuePair(key, value);
assertTrue(params.contains(item), params + " should contain " + item);
}
}
Loading

0 comments on commit 9ce9554

Please sign in to comment.