From 3a8a7bc2114eeaadb06faf2426633ba578334a3b Mon Sep 17 00:00:00 2001 From: Danielle Vass Date: Mon, 6 Apr 2020 10:28:18 +0100 Subject: [PATCH 01/12] Add. prepareEvent method to InternalChannel interface and implement in ChannelImpl --- .../client/channel/impl/ChannelImpl.java | 23 ++++++++++++------- .../client/channel/impl/InternalChannel.java | 3 +++ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/pusher/client/channel/impl/ChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/ChannelImpl.java index 4ae849b6..97d44a95 100644 --- a/src/main/java/com/pusher/client/channel/impl/ChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/ChannelImpl.java @@ -89,6 +89,11 @@ public boolean isSubscribed() { /* InternalChannel implementation */ + @Override + public PusherEvent prepareEvent(String event, String message) { + return GSON.fromJson(message, PusherEvent.class); + } + @Override public void onMessage(final String event, final String message) { @@ -108,14 +113,16 @@ public void onMessage(final String event, final String message) { } if (listeners != null) { - for (final SubscriptionEventListener listener : listeners) { - final PusherEvent e = GSON.fromJson(message, PusherEvent.class); - factory.queueOnEventThread(new Runnable() { - @Override - public void run() { - listener.onEvent(e); - } - }); + final PusherEvent pusherEvent = prepareEvent(event, message); + if (pusherEvent != null) { + for (final SubscriptionEventListener listener : listeners) { + factory.queueOnEventThread(new Runnable() { + @Override + public void run() { + listener.onEvent(pusherEvent); + } + }); + } } } } diff --git a/src/main/java/com/pusher/client/channel/impl/InternalChannel.java b/src/main/java/com/pusher/client/channel/impl/InternalChannel.java index e1668b7a..b6d8e032 100644 --- a/src/main/java/com/pusher/client/channel/impl/InternalChannel.java +++ b/src/main/java/com/pusher/client/channel/impl/InternalChannel.java @@ -3,6 +3,7 @@ import com.pusher.client.channel.Channel; import com.pusher.client.channel.ChannelEventListener; import com.pusher.client.channel.ChannelState; +import com.pusher.client.channel.PusherEvent; public interface InternalChannel extends Channel, Comparable { @@ -10,6 +11,8 @@ public interface InternalChannel extends Channel, Comparable { String toUnsubscribeMessage(); + PusherEvent prepareEvent(String event, String message); + void onMessage(String event, String message); void updateState(ChannelState state); From c8ea300047500d57dc8823088c92fa02ecc4b3df Mon Sep 17 00:00:00 2001 From: Danielle Vass Date: Mon, 6 Apr 2020 10:45:33 +0100 Subject: [PATCH 02/12] Refactor getInterestedListeners to it's own method in ChannelImpl --- .../client/channel/impl/ChannelImpl.java | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/pusher/client/channel/impl/ChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/ChannelImpl.java index 97d44a95..8bb49e46 100644 --- a/src/main/java/com/pusher/client/channel/impl/ChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/ChannelImpl.java @@ -99,19 +99,8 @@ public void onMessage(final String event, final String message) { if (event.equals(SUBSCRIPTION_SUCCESS_EVENT)) { updateState(ChannelState.SUBSCRIBED); - } - else { - final Set listeners; - synchronized (lock) { - final Set sharedListeners = eventNameToListenerMap.get(event); - if (sharedListeners != null) { - listeners = new HashSet(sharedListeners); - } - else { - listeners = null; - } - } - + } else { + final Set listeners = getInterestedListeners(event); if (listeners != null) { final PusherEvent pusherEvent = prepareEvent(event, message); if (pusherEvent != null) { @@ -220,4 +209,17 @@ private void validateArguments(final String eventName, final SubscriptionEventLi "Cannot bind or unbind to events on a channel that has been unsubscribed. Call Pusher.subscribe() to resubscribe to this channel"); } } + + protected Set getInterestedListeners(String event) { + final Set listeners; + synchronized (lock) { + final Set sharedListeners = eventNameToListenerMap.get(event); + if (sharedListeners != null) { + listeners = new HashSet(sharedListeners); + } else { + listeners = null; + } + } + return listeners; + } } From f290cb6f60667a05678329b72a6c8cb0717342b7 Mon Sep 17 00:00:00 2001 From: Danielle Vass Date: Mon, 6 Apr 2020 10:48:09 +0100 Subject: [PATCH 03/12] Decrypt PrivateEncrypted messages and retry once --- .../PrivateEncryptedChannelEventListener.java | 2 +- .../impl/PrivateEncryptedChannelImpl.java | 70 ++++++++++++++++++ .../PrivateEncryptedChannelExampleApp.java | 6 ++ .../impl/PrivateEncryptedChannelImplTest.java | 74 +++++++++++++++++++ 4 files changed, 151 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/pusher/client/channel/PrivateEncryptedChannelEventListener.java b/src/main/java/com/pusher/client/channel/PrivateEncryptedChannelEventListener.java index a6aeeaae..3994dc83 100644 --- a/src/main/java/com/pusher/client/channel/PrivateEncryptedChannelEventListener.java +++ b/src/main/java/com/pusher/client/channel/PrivateEncryptedChannelEventListener.java @@ -6,5 +6,5 @@ */ public interface PrivateEncryptedChannelEventListener extends PrivateChannelEventListener { - // TODO: add onDecryptionFailure + void onDecryptionFailure(Exception e); } diff --git a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java index 2d54cd85..5d5d75ce 100644 --- a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java @@ -5,18 +5,22 @@ import com.pusher.client.channel.ChannelState; import com.pusher.client.channel.PrivateEncryptedChannel; import com.pusher.client.channel.PrivateEncryptedChannelEventListener; +import com.pusher.client.channel.PusherEvent; import com.pusher.client.channel.SubscriptionEventListener; import com.pusher.client.connection.ConnectionEventListener; import com.pusher.client.connection.ConnectionState; import com.pusher.client.connection.ConnectionStateChange; import com.pusher.client.connection.impl.InternalConnection; +import com.pusher.client.crypto.nacl.AuthenticityException; import com.pusher.client.crypto.nacl.SecretBoxOpener; import com.pusher.client.crypto.nacl.SecretBoxOpenerFactory; import com.pusher.client.util.Factory; import com.pusher.client.util.internal.Base64; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; +import java.util.Set; public class PrivateEncryptedChannelImpl extends ChannelImpl implements PrivateEncryptedChannel { @@ -124,6 +128,72 @@ public void updateState(ChannelState state) { } } + @Override + public PusherEvent prepareEvent(String event, String message) { + try { + + Map receivedMessage = GSON.fromJson(message, Map.class); + final String decryptedMessage = decryptMessage((String) receivedMessage.get("data")); + receivedMessage.replace("data", decryptedMessage); + + return GSON.fromJson( + GSON.toJson(receivedMessage), PusherEvent.class); + + } catch (AuthenticityException e1) { + + // retry once only. + disposeSecretBoxOpener(); + authenticate(); + + try { + Map receivedMessage = GSON.fromJson(message, Map.class); + final String decryptedMessage = decryptMessage((String) receivedMessage.get("data")); + receivedMessage.replace("data", decryptedMessage); + + return GSON.fromJson( + GSON.toJson(receivedMessage), PusherEvent.class); + } catch (AuthenticityException e2) { + disposeSecretBoxOpener(); + notifyListenersOfDecryptFailure(event); + } + } + + return null; + } + + private void notifyListenersOfDecryptFailure(final String event) { + Set listeners = getInterestedListeners(event); + if (listeners != null) { + for (SubscriptionEventListener listener : listeners) { + ((PrivateEncryptedChannelEventListener)listener).onDecryptionFailure( + new Exception("Failed to decrypt message")); + } + } + } + + private class EncryptedReceivedData { + String nonce; + String ciphertext; + + public byte[] getNonce() { + return Base64.decode(nonce); + } + + public byte[] getCiphertext() { + return Base64.decode(ciphertext); + } + } + + private String decryptMessage(String data) { + + final EncryptedReceivedData encryptedReceivedData = + GSON.fromJson(data, EncryptedReceivedData.class); + + return new String(secretBoxOpener.open( + encryptedReceivedData.getCiphertext(), + encryptedReceivedData.getNonce())); + } + private void disposeSecretBoxOpener() { if (secretBoxOpener != null) { secretBoxOpener.clearKey(); diff --git a/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java b/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java index b883e60f..24d2feb3 100644 --- a/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java +++ b/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java @@ -86,4 +86,10 @@ public void onError(String message, String code, Exception e) { code, e)); } + + @Override + public void onDecryptionFailure(Exception e) { + System.out.println(String.format( + "An error was received decrypting message - exception: [%s]", e)); + } } diff --git a/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java b/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java index d9e639bb..88e1d224 100644 --- a/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java +++ b/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java @@ -1,9 +1,12 @@ package com.pusher.client.channel.impl; import static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.pusher.client.AuthorizationFailureException; @@ -11,7 +14,10 @@ import com.pusher.client.channel.ChannelEventListener; import com.pusher.client.channel.PrivateEncryptedChannelEventListener; import com.pusher.client.connection.impl.InternalConnection; +import com.pusher.client.crypto.nacl.SecretBoxOpener; import com.pusher.client.crypto.nacl.SecretBoxOpenerFactory; +import com.pusher.client.util.internal.Base64; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -26,6 +32,7 @@ public class PrivateEncryptedChannelImplTest extends ChannelImplTest { final String AUTH_RESPONSE_MISSING_AUTH = "{\"shared_secret\":\"iBvNoPVYwByqSfg6anjPpEQ2j051b3rt1Vmnb+z5doo=\"}"; final String AUTH_RESPONSE_MISSING_SHARED_SECRET = "{\"auth\":\"636a81ba7e7b15725c00:3ee04892514e8a669dc5d30267221f16727596688894712cad305986e6fc0f3c\"}"; final String AUTH_RESPONSE_INVALID_JSON = "potatoes"; + final String SHARED_SECRET = "iBvNoPVYwByqSfg6anjPpEQ2j051b3rt1Vmnb+z5doo="; @Mock InternalConnection mockInternalConnection; @@ -148,4 +155,71 @@ public void authenticationThrowsExceptionIfMalformedJson() { channel.toSubscribeMessage(); } + + /* + ON MESSAGE + */ + @Test + public void testDataIsExtractedFromMessageAndPassedToSingleListener() { + PrivateEncryptedChannelImpl channel = new PrivateEncryptedChannelImpl( + mockInternalConnection, + getChannelName(), + mockAuthorizer, + factory, + mockSecretBoxOpenerFactory); + + when(mockAuthorizer.authorize(Matchers.anyString(), Matchers.anyString())) + .thenReturn(AUTH_RESPONSE); + when(mockSecretBoxOpenerFactory.create(any())) + .thenReturn(new SecretBoxOpener(Base64.decode(SHARED_SECRET))); + + channel.toSubscribeMessage(); + + PrivateEncryptedChannelEventListener mockListener = mock(PrivateEncryptedChannelEventListener.class); + + channel.bind("my-event", mockListener); + channel.onMessage("my-event", "{\"event\":\"event1\",\"data\":\"{" + + "\\\"nonce\\\": \\\"4sVYwy4j/8dCcjyxtPCWyk19GaaViaW9\\\"," + + "\\\"ciphertext\\\": \\\"/GMESnFGlbNn01BuBjp31XYa3i9vZsGKR8fgR9EDhXKx3lzGiUD501A=\\\"" + + "}\"}"); + + verify(mockListener, times(1)).onEvent(argCaptor.capture()); + assertEquals("event1", argCaptor.getValue().getEventName()); + assertEquals("{\"message\":\"hello world\"}", argCaptor.getValue().getData()); + } + + @Test + public void testDataIsExtractedFromMessageAndPassedToMultipleListeners() { + PrivateEncryptedChannelImpl channel = new PrivateEncryptedChannelImpl( + mockInternalConnection, + getChannelName(), + mockAuthorizer, + factory, + mockSecretBoxOpenerFactory); + + when(mockAuthorizer.authorize(Matchers.anyString(), Matchers.anyString())) + .thenReturn(AUTH_RESPONSE); + when(mockSecretBoxOpenerFactory.create(any())) + .thenReturn(new SecretBoxOpener(Base64.decode(SHARED_SECRET))); + + channel.toSubscribeMessage(); + + PrivateEncryptedChannelEventListener mockListener1 = mock(PrivateEncryptedChannelEventListener.class); + PrivateEncryptedChannelEventListener mockListener2 = mock(PrivateEncryptedChannelEventListener.class); + + channel.bind("my-event", mockListener1); + channel.bind("my-event", mockListener2); + channel.onMessage("my-event", "{\"event\":\"event1\",\"data\":\"{" + + "\\\"nonce\\\": \\\"4sVYwy4j/8dCcjyxtPCWyk19GaaViaW9\\\"," + + "\\\"ciphertext\\\": \\\"/GMESnFGlbNn01BuBjp31XYa3i9vZsGKR8fgR9EDhXKx3lzGiUD501A=\\\"" + + "}\"}"); + + verify(mockListener1).onEvent(argCaptor.capture()); + assertEquals("event1", argCaptor.getValue().getEventName()); + assertEquals("{\"message\":\"hello world\"}", argCaptor.getValue().getData()); + + verify(mockListener2).onEvent(argCaptor.capture()); + assertEquals("event1", argCaptor.getValue().getEventName()); + assertEquals("{\"message\":\"hello world\"}", argCaptor.getValue().getData()); + } } From 0cf29b77af9f2192966cc3dc5e32fec56a93fccc Mon Sep 17 00:00:00 2001 From: Danielle Vass Date: Mon, 6 Apr 2020 14:10:12 +0100 Subject: [PATCH 04/12] Add tests for retrying decrypting messages --- .../impl/PrivateEncryptedChannelImpl.java | 1 - .../impl/PrivateEncryptedChannelImplTest.java | 66 +++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java index 5d5d75ce..ba60cb93 100644 --- a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java @@ -17,7 +17,6 @@ import com.pusher.client.util.Factory; import com.pusher.client.util.internal.Base64; -import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; diff --git a/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java b/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java index 88e1d224..81b7590f 100644 --- a/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java +++ b/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java @@ -21,6 +21,8 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; import org.mockito.Matchers; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; @@ -33,6 +35,8 @@ public class PrivateEncryptedChannelImplTest extends ChannelImplTest { final String AUTH_RESPONSE_MISSING_SHARED_SECRET = "{\"auth\":\"636a81ba7e7b15725c00:3ee04892514e8a669dc5d30267221f16727596688894712cad305986e6fc0f3c\"}"; final String AUTH_RESPONSE_INVALID_JSON = "potatoes"; final String SHARED_SECRET = "iBvNoPVYwByqSfg6anjPpEQ2j051b3rt1Vmnb+z5doo="; + final String AUTH_RESPONSE_INCORRECT_SHARED_SECRET = "{\"auth\":\"636a81ba7e7b15725c00:3ee04892514e8a669dc5d30267221f16727596688894712cad305986e6fc0f3c\",\"shared_secret\":\"iBvNoPVYwByqSfg6anjPpEQ2j051b3rt1Vmnb+z5do0=\"}"; + final String SHARED_SECRET_INCORRECT = "iBvNoPVYwByqSfg6anjPpEQ2j051b3rt1Vmnb+z5do0="; @Mock InternalConnection mockInternalConnection; @@ -41,6 +45,9 @@ public class PrivateEncryptedChannelImplTest extends ChannelImplTest { @Mock SecretBoxOpenerFactory mockSecretBoxOpenerFactory; + @Captor + ArgumentCaptor exceptionArgumentCaptor; + @Override @Before public void setUp() { @@ -222,4 +229,63 @@ public void testDataIsExtractedFromMessageAndPassedToMultipleListeners() { assertEquals("event1", argCaptor.getValue().getEventName()); assertEquals("{\"message\":\"hello world\"}", argCaptor.getValue().getData()); } + + @Test + public void onMessageRaisesExceptionWhenFailingToDecryptTwice() { + PrivateEncryptedChannelImpl channel = new PrivateEncryptedChannelImpl( + mockInternalConnection, + getChannelName(), + mockAuthorizer, + factory, + mockSecretBoxOpenerFactory); + + when(mockAuthorizer.authorize(Matchers.anyString(), Matchers.anyString())) + .thenReturn(AUTH_RESPONSE_INCORRECT_SHARED_SECRET) + .thenReturn(AUTH_RESPONSE_INCORRECT_SHARED_SECRET); + when(mockSecretBoxOpenerFactory.create(any())) + .thenReturn(new SecretBoxOpener(Base64.decode(SHARED_SECRET_INCORRECT))) + .thenReturn(new SecretBoxOpener(Base64.decode(SHARED_SECRET_INCORRECT))); + + channel.toSubscribeMessage(); + + PrivateEncryptedChannelEventListener mockListener1 = mock(PrivateEncryptedChannelEventListener.class); + channel.bind("my-event", mockListener1); + channel.onMessage("my-event", "{\"event\":\"event1\",\"data\":\"{" + + "\\\"nonce\\\": \\\"4sVYwy4j/8dCcjyxtPCWyk19GaaViaW9\\\"," + + "\\\"ciphertext\\\": \\\"/GMESnFGlbNn01BuBjp31XYa3i9vZsGKR8fgR9EDhXKx3lzGiUD501A=\\\"" + + "}\"}"); + + verify(mockListener1).onDecryptionFailure(exceptionArgumentCaptor.capture()); + } + + @Test + public void onMessageRetriesDecryptionOnce() { + PrivateEncryptedChannelImpl channel = new PrivateEncryptedChannelImpl( + mockInternalConnection, + getChannelName(), + mockAuthorizer, + factory, + mockSecretBoxOpenerFactory); + + when(mockAuthorizer.authorize(Matchers.anyString(), Matchers.anyString())) + .thenReturn(AUTH_RESPONSE_INCORRECT_SHARED_SECRET) + .thenReturn(AUTH_RESPONSE); + when(mockSecretBoxOpenerFactory.create(any())) + .thenReturn(new SecretBoxOpener(Base64.decode(SHARED_SECRET_INCORRECT))) + .thenReturn(new SecretBoxOpener(Base64.decode(SHARED_SECRET))); + + channel.toSubscribeMessage(); + + PrivateEncryptedChannelEventListener mockListener1 = mock(PrivateEncryptedChannelEventListener.class); + channel.bind("my-event", mockListener1); + channel.onMessage("my-event", "{\"event\":\"event1\",\"data\":\"{" + + "\\\"nonce\\\": \\\"4sVYwy4j/8dCcjyxtPCWyk19GaaViaW9\\\"," + + "\\\"ciphertext\\\": \\\"/GMESnFGlbNn01BuBjp31XYa3i9vZsGKR8fgR9EDhXKx3lzGiUD501A=\\\"" + + "}\"}"); + + verify(mockListener1).onEvent(argCaptor.capture()); + assertEquals("event1", argCaptor.getValue().getEventName()); + assertEquals("{\"message\":\"hello world\"}", argCaptor.getValue().getData()); + } + } From 0e178e8ab08fcad4c3f340a6b4f431b889991b84 Mon Sep 17 00:00:00 2001 From: Danielle Vass Date: Mon, 6 Apr 2020 16:09:26 +0100 Subject: [PATCH 05/12] Simplify the getInterestedListeners method in ChannelImpl --- .../java/com/pusher/client/channel/impl/ChannelImpl.java | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/main/java/com/pusher/client/channel/impl/ChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/ChannelImpl.java index 8bb49e46..157fc1c2 100644 --- a/src/main/java/com/pusher/client/channel/impl/ChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/ChannelImpl.java @@ -211,15 +211,8 @@ private void validateArguments(final String eventName, final SubscriptionEventLi } protected Set getInterestedListeners(String event) { - final Set listeners; synchronized (lock) { - final Set sharedListeners = eventNameToListenerMap.get(event); - if (sharedListeners != null) { - listeners = new HashSet(sharedListeners); - } else { - listeners = null; - } + return eventNameToListenerMap.get(event); } - return listeners; } } From 4bfc70de174c9abae8c6a012e29a2507c3570fd9 Mon Sep 17 00:00:00 2001 From: Danielle Vass Date: Mon, 6 Apr 2020 16:29:59 +0100 Subject: [PATCH 06/12] Handle multiple failed decryption calls better --- .../PrivateEncryptedChannelEventListener.java | 2 +- .../impl/PrivateEncryptedChannelImpl.java | 42 ++++++------ .../PrivateEncryptedChannelExampleApp.java | 4 +- .../impl/PrivateEncryptedChannelImplTest.java | 64 ++++++++++++++----- 4 files changed, 76 insertions(+), 36 deletions(-) diff --git a/src/main/java/com/pusher/client/channel/PrivateEncryptedChannelEventListener.java b/src/main/java/com/pusher/client/channel/PrivateEncryptedChannelEventListener.java index 3994dc83..119fc58a 100644 --- a/src/main/java/com/pusher/client/channel/PrivateEncryptedChannelEventListener.java +++ b/src/main/java/com/pusher/client/channel/PrivateEncryptedChannelEventListener.java @@ -6,5 +6,5 @@ */ public interface PrivateEncryptedChannelEventListener extends PrivateChannelEventListener { - void onDecryptionFailure(Exception e); + void onDecryptionFailure(String event, String reason); } diff --git a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java index ba60cb93..e9844dfd 100644 --- a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java @@ -129,43 +129,49 @@ public void updateState(ChannelState state) { @Override public PusherEvent prepareEvent(String event, String message) { - try { - - Map receivedMessage = GSON.fromJson(message, Map.class); - final String decryptedMessage = decryptMessage((String) receivedMessage.get("data")); - receivedMessage.replace("data", decryptedMessage); - - return GSON.fromJson( - GSON.toJson(receivedMessage), PusherEvent.class); - } catch (AuthenticityException e1) { - - // retry once only. - disposeSecretBoxOpener(); - authenticate(); + if (secretBoxOpener == null) { + notifyListenersOfDecryptFailure(event, "Too many failed attempts to decrypt a previous message."); + } else { try { + Map receivedMessage = GSON.fromJson(message, Map.class); final String decryptedMessage = decryptMessage((String) receivedMessage.get("data")); receivedMessage.replace("data", decryptedMessage); return GSON.fromJson( GSON.toJson(receivedMessage), PusherEvent.class); - } catch (AuthenticityException e2) { + + } catch (AuthenticityException e1) { + + // retry once only. disposeSecretBoxOpener(); - notifyListenersOfDecryptFailure(event); + authenticate(); + + try { + Map receivedMessage = GSON.fromJson(message, Map.class); + final String decryptedMessage = decryptMessage((String) receivedMessage.get("data")); + receivedMessage.replace("data", decryptedMessage); + + return GSON.fromJson( + GSON.toJson(receivedMessage), PusherEvent.class); + } catch (AuthenticityException e2) { + disposeSecretBoxOpener(); + notifyListenersOfDecryptFailure(event, "Failed to decrypt message."); + } } + } - return null; } - private void notifyListenersOfDecryptFailure(final String event) { + private void notifyListenersOfDecryptFailure(final String event, final String reason) { Set listeners = getInterestedListeners(event); if (listeners != null) { for (SubscriptionEventListener listener : listeners) { ((PrivateEncryptedChannelEventListener)listener).onDecryptionFailure( - new Exception("Failed to decrypt message")); + event, reason); } } } diff --git a/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java b/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java index 24d2feb3..28f4a6ff 100644 --- a/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java +++ b/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java @@ -88,8 +88,8 @@ public void onError(String message, String code, Exception e) { } @Override - public void onDecryptionFailure(Exception e) { + public void onDecryptionFailure(String event, String reason) { System.out.println(String.format( - "An error was received decrypting message - exception: [%s]", e)); + "An error was received decrypting message for event:[%s] - reason: [%s]", event, reason)); } } diff --git a/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java b/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java index 81b7590f..5b781259 100644 --- a/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java +++ b/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java @@ -1,14 +1,5 @@ package com.pusher.client.channel.impl; -import static org.junit.Assert.assertEquals; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyString; -import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - import com.pusher.client.AuthorizationFailureException; import com.pusher.client.Authorizer; import com.pusher.client.channel.ChannelEventListener; @@ -21,12 +12,19 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.ArgumentCaptor; -import org.mockito.Captor; import org.mockito.Matchers; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; +import static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + @RunWith(MockitoJUnitRunner.class) public class PrivateEncryptedChannelImplTest extends ChannelImplTest { @@ -45,9 +43,6 @@ public class PrivateEncryptedChannelImplTest extends ChannelImplTest { @Mock SecretBoxOpenerFactory mockSecretBoxOpenerFactory; - @Captor - ArgumentCaptor exceptionArgumentCaptor; - @Override @Before public void setUp() { @@ -255,7 +250,7 @@ public void onMessageRaisesExceptionWhenFailingToDecryptTwice() { "\\\"ciphertext\\\": \\\"/GMESnFGlbNn01BuBjp31XYa3i9vZsGKR8fgR9EDhXKx3lzGiUD501A=\\\"" + "}\"}"); - verify(mockListener1).onDecryptionFailure(exceptionArgumentCaptor.capture()); + verify(mockListener1).onDecryptionFailure(anyString(), anyString()); } @Test @@ -288,4 +283,43 @@ public void onMessageRetriesDecryptionOnce() { assertEquals("{\"message\":\"hello world\"}", argCaptor.getValue().getData()); } + @Test + public void twoEventsReceivedWithIncorrectSharedSecret() { + + PrivateEncryptedChannelImpl channel = new PrivateEncryptedChannelImpl( + mockInternalConnection, + getChannelName(), + mockAuthorizer, + factory, + mockSecretBoxOpenerFactory); + + when(mockAuthorizer.authorize(Matchers.anyString(), Matchers.anyString())) + .thenReturn(AUTH_RESPONSE_INCORRECT_SHARED_SECRET) + .thenReturn(AUTH_RESPONSE_INCORRECT_SHARED_SECRET) + .thenReturn(AUTH_RESPONSE_INCORRECT_SHARED_SECRET); + when(mockSecretBoxOpenerFactory.create(any())) + .thenReturn(new SecretBoxOpener(Base64.decode(SHARED_SECRET_INCORRECT))) + .thenReturn(new SecretBoxOpener(Base64.decode(SHARED_SECRET_INCORRECT))) + .thenReturn(new SecretBoxOpener(Base64.decode(SHARED_SECRET_INCORRECT))); + + channel.toSubscribeMessage(); + + PrivateEncryptedChannelEventListener mockListener1 = mock(PrivateEncryptedChannelEventListener.class); + channel.bind("my-event", mockListener1); + channel.onMessage("my-event", "{\"event\":\"event1\",\"data\":\"{" + + "\\\"nonce\\\": \\\"4sVYwy4j/8dCcjyxtPCWyk19GaaViaW9\\\"," + + "\\\"ciphertext\\\": \\\"/GMESnFGlbNn01BuBjp31XYa3i9vZsGKR8fgR9EDhXKx3lzGiUD501A=\\\"" + + "}\"}"); + + verify(mockListener1).onDecryptionFailure("my-event", "Failed to decrypt message."); + + // send a second message + channel.onMessage("my-event", "{\"event\":\"event1\",\"data\":\"{" + + "\\\"nonce\\\": \\\"4sVYwy4j/8dCcjyxtPCWyk19GaaViaW9\\\"," + + "\\\"ciphertext\\\": \\\"/GMESnFGlbNn01BuBjp31XYa3i9vZsGKR8fgR9EDhXKx3lzGiUD501A=\\\"" + + "}\"}"); + + verify(mockListener1).onDecryptionFailure("my-event", "Too many failed attempts to decrypt a previous message."); + } + } From 3a8f63ecf0527ad80fc50dbcda7ddbb76a1510c5 Mon Sep 17 00:00:00 2001 From: Danielle Vass Date: Mon, 6 Apr 2020 16:40:34 +0100 Subject: [PATCH 07/12] When decrypting a message, pass the json map to the PusherEvent constructor --- .../channel/impl/PrivateEncryptedChannelImpl.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java index e9844dfd..269442f4 100644 --- a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java @@ -136,12 +136,12 @@ public PusherEvent prepareEvent(String event, String message) { try { - Map receivedMessage = GSON.fromJson(message, Map.class); + Map receivedMessage = + GSON.>fromJson(message, Map.class); final String decryptedMessage = decryptMessage((String) receivedMessage.get("data")); receivedMessage.replace("data", decryptedMessage); - return GSON.fromJson( - GSON.toJson(receivedMessage), PusherEvent.class); + return new PusherEvent(receivedMessage); } catch (AuthenticityException e1) { @@ -150,12 +150,12 @@ public PusherEvent prepareEvent(String event, String message) { authenticate(); try { - Map receivedMessage = GSON.fromJson(message, Map.class); + Map receivedMessage = + GSON.>fromJson(message, Map.class); final String decryptedMessage = decryptMessage((String) receivedMessage.get("data")); receivedMessage.replace("data", decryptedMessage); - return GSON.fromJson( - GSON.toJson(receivedMessage), PusherEvent.class); + return new PusherEvent(receivedMessage); } catch (AuthenticityException e2) { disposeSecretBoxOpener(); notifyListenersOfDecryptFailure(event, "Failed to decrypt message."); From b03e46690292a35b8b81066280e4257f2348d6fc Mon Sep 17 00:00:00 2001 From: Danielle Vass Date: Mon, 6 Apr 2020 16:47:40 +0100 Subject: [PATCH 08/12] Refactor decryptMessage to package up as a PusherEvent --- .../impl/PrivateEncryptedChannelImpl.java | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java index 269442f4..c50a1d3b 100644 --- a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java @@ -135,14 +135,7 @@ public PusherEvent prepareEvent(String event, String message) { } else { try { - - Map receivedMessage = - GSON.>fromJson(message, Map.class); - final String decryptedMessage = decryptMessage((String) receivedMessage.get("data")); - receivedMessage.replace("data", decryptedMessage); - - return new PusherEvent(receivedMessage); - + return decryptMessage(message); } catch (AuthenticityException e1) { // retry once only. @@ -150,12 +143,7 @@ public PusherEvent prepareEvent(String event, String message) { authenticate(); try { - Map receivedMessage = - GSON.>fromJson(message, Map.class); - final String decryptedMessage = decryptMessage((String) receivedMessage.get("data")); - receivedMessage.replace("data", decryptedMessage); - - return new PusherEvent(receivedMessage); + return decryptMessage(message); } catch (AuthenticityException e2) { disposeSecretBoxOpener(); notifyListenersOfDecryptFailure(event, "Failed to decrypt message."); @@ -189,14 +177,21 @@ public byte[] getCiphertext() { } } - private String decryptMessage(String data) { + private PusherEvent decryptMessage(String message) { + + Map receivedMessage = + GSON.>fromJson(message, Map.class); final EncryptedReceivedData encryptedReceivedData = - GSON.fromJson(data, EncryptedReceivedData.class); + GSON.fromJson((String)receivedMessage.get("data"), EncryptedReceivedData.class); - return new String(secretBoxOpener.open( + String decryptedData = new String(secretBoxOpener.open( encryptedReceivedData.getCiphertext(), encryptedReceivedData.getNonce())); + + receivedMessage.replace("data", decryptedData); + + return new PusherEvent(receivedMessage); } private void disposeSecretBoxOpener() { From a87547f6940ff3f6482bcba7956ebfd476470e1d Mon Sep 17 00:00:00 2001 From: Danielle Vass Date: Mon, 6 Apr 2020 16:51:14 +0100 Subject: [PATCH 09/12] Move the retry logic into it's own method --- .../impl/PrivateEncryptedChannelImpl.java | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java index c50a1d3b..f5d5f845 100644 --- a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java @@ -138,22 +138,27 @@ public PusherEvent prepareEvent(String event, String message) { return decryptMessage(message); } catch (AuthenticityException e1) { - // retry once only. - disposeSecretBoxOpener(); - authenticate(); - - try { - return decryptMessage(message); - } catch (AuthenticityException e2) { - disposeSecretBoxOpener(); - notifyListenersOfDecryptFailure(event, "Failed to decrypt message."); - } + retryDecryptingMessage(event, message); } } return null; } + private PusherEvent retryDecryptingMessage(String event, String message) { + + disposeSecretBoxOpener(); + authenticate(); + + try { + return decryptMessage(message); + } catch (AuthenticityException e2) { + disposeSecretBoxOpener(); + notifyListenersOfDecryptFailure(event, "Failed to decrypt message."); + } + return null; + } + private void notifyListenersOfDecryptFailure(final String event, final String reason) { Set listeners = getInterestedListeners(event); if (listeners != null) { From 2f5b7e27aae3eec3182faa9678d00f6ae2532747 Mon Sep 17 00:00:00 2001 From: Danielle Vass Date: Mon, 6 Apr 2020 16:54:25 +0100 Subject: [PATCH 10/12] Revert "Move the retry logic into it's own method" This reverts commit a87547f6940ff3f6482bcba7956ebfd476470e1d. --- .../impl/PrivateEncryptedChannelImpl.java | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java index f5d5f845..c50a1d3b 100644 --- a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java @@ -138,27 +138,22 @@ public PusherEvent prepareEvent(String event, String message) { return decryptMessage(message); } catch (AuthenticityException e1) { - retryDecryptingMessage(event, message); + // retry once only. + disposeSecretBoxOpener(); + authenticate(); + + try { + return decryptMessage(message); + } catch (AuthenticityException e2) { + disposeSecretBoxOpener(); + notifyListenersOfDecryptFailure(event, "Failed to decrypt message."); + } } } return null; } - private PusherEvent retryDecryptingMessage(String event, String message) { - - disposeSecretBoxOpener(); - authenticate(); - - try { - return decryptMessage(message); - } catch (AuthenticityException e2) { - disposeSecretBoxOpener(); - notifyListenersOfDecryptFailure(event, "Failed to decrypt message."); - } - return null; - } - private void notifyListenersOfDecryptFailure(final String event, final String reason) { Set listeners = getInterestedListeners(event); if (listeners != null) { From 0a62b935af356bed5f263264257eca86ce5b8950 Mon Sep 17 00:00:00 2001 From: Danielle Vass Date: Tue, 7 Apr 2020 09:59:02 +0100 Subject: [PATCH 11/12] Keep the shared_secret after the second retry so any subsequent messages get a retry to the authorization endpoint --- .../impl/PrivateEncryptedChannelImpl.java | 28 +++++------ .../impl/PrivateEncryptedChannelImplTest.java | 47 +++++++++++++++++-- 2 files changed, 55 insertions(+), 20 deletions(-) diff --git a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java index c50a1d3b..66a9c329 100644 --- a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java @@ -130,27 +130,23 @@ public void updateState(ChannelState state) { @Override public PusherEvent prepareEvent(String event, String message) { - if (secretBoxOpener == null) { - notifyListenersOfDecryptFailure(event, "Too many failed attempts to decrypt a previous message."); - } else { + try { + return decryptMessage(message); + } catch (AuthenticityException e1) { + + // retry once only. + disposeSecretBoxOpener(); + authenticate(); try { return decryptMessage(message); - } catch (AuthenticityException e1) { - - // retry once only. - disposeSecretBoxOpener(); - authenticate(); - - try { - return decryptMessage(message); - } catch (AuthenticityException e2) { - disposeSecretBoxOpener(); - notifyListenersOfDecryptFailure(event, "Failed to decrypt message."); - } + } catch (AuthenticityException e2) { + // deliberately not destroying the secretBoxOpener so the next message + // has an opportunity to fetch a new key and decrypt + notifyListenersOfDecryptFailure(event, "Failed to decrypt message."); } - } + return null; } diff --git a/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java b/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java index 5b781259..c11eb6ce 100644 --- a/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java +++ b/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java @@ -284,7 +284,7 @@ public void onMessageRetriesDecryptionOnce() { } @Test - public void twoEventsReceivedWithIncorrectSharedSecret() { + public void twoEventsReceivedWithSecondRetryCorrect() { PrivateEncryptedChannelImpl channel = new PrivateEncryptedChannelImpl( mockInternalConnection, @@ -296,11 +296,11 @@ public void twoEventsReceivedWithIncorrectSharedSecret() { when(mockAuthorizer.authorize(Matchers.anyString(), Matchers.anyString())) .thenReturn(AUTH_RESPONSE_INCORRECT_SHARED_SECRET) .thenReturn(AUTH_RESPONSE_INCORRECT_SHARED_SECRET) - .thenReturn(AUTH_RESPONSE_INCORRECT_SHARED_SECRET); + .thenReturn(AUTH_RESPONSE); when(mockSecretBoxOpenerFactory.create(any())) .thenReturn(new SecretBoxOpener(Base64.decode(SHARED_SECRET_INCORRECT))) .thenReturn(new SecretBoxOpener(Base64.decode(SHARED_SECRET_INCORRECT))) - .thenReturn(new SecretBoxOpener(Base64.decode(SHARED_SECRET_INCORRECT))); + .thenReturn(new SecretBoxOpener(Base64.decode(SHARED_SECRET))); channel.toSubscribeMessage(); @@ -319,7 +319,46 @@ public void twoEventsReceivedWithIncorrectSharedSecret() { "\\\"ciphertext\\\": \\\"/GMESnFGlbNn01BuBjp31XYa3i9vZsGKR8fgR9EDhXKx3lzGiUD501A=\\\"" + "}\"}"); - verify(mockListener1).onDecryptionFailure("my-event", "Too many failed attempts to decrypt a previous message."); + verify(mockListener1).onEvent(argCaptor.capture()); + assertEquals("event1", argCaptor.getValue().getEventName()); + assertEquals("{\"message\":\"hello world\"}", argCaptor.getValue().getData()); + } + + @Test + public void twoEventsReceivedWithIncorrectSharedSecret() { + + PrivateEncryptedChannelImpl channel = new PrivateEncryptedChannelImpl( + mockInternalConnection, + getChannelName(), + mockAuthorizer, + factory, + mockSecretBoxOpenerFactory); + + when(mockAuthorizer.authorize(Matchers.anyString(), Matchers.anyString())) + .thenReturn(AUTH_RESPONSE_INCORRECT_SHARED_SECRET) + .thenReturn(AUTH_RESPONSE_INCORRECT_SHARED_SECRET) + .thenReturn(AUTH_RESPONSE_INCORRECT_SHARED_SECRET); + when(mockSecretBoxOpenerFactory.create(any())) + .thenReturn(new SecretBoxOpener(Base64.decode(SHARED_SECRET_INCORRECT))) + .thenReturn(new SecretBoxOpener(Base64.decode(SHARED_SECRET_INCORRECT))) + .thenReturn(new SecretBoxOpener(Base64.decode(SHARED_SECRET_INCORRECT))); + + channel.toSubscribeMessage(); + + PrivateEncryptedChannelEventListener mockListener1 = mock(PrivateEncryptedChannelEventListener.class); + channel.bind("my-event", mockListener1); + channel.onMessage("my-event", "{\"event\":\"event1\",\"data\":\"{" + + "\\\"nonce\\\": \\\"4sVYwy4j/8dCcjyxtPCWyk19GaaViaW9\\\"," + + "\\\"ciphertext\\\": \\\"/GMESnFGlbNn01BuBjp31XYa3i9vZsGKR8fgR9EDhXKx3lzGiUD501A=\\\"" + + "}\"}"); + // send a second message + channel.onMessage("my-event", "{\"event\":\"event1\",\"data\":\"{" + + "\\\"nonce\\\": \\\"4sVYwy4j/8dCcjyxtPCWyk19GaaViaW9\\\"," + + "\\\"ciphertext\\\": \\\"/GMESnFGlbNn01BuBjp31XYa3i9vZsGKR8fgR9EDhXKx3lzGiUD501A=\\\"" + + "}\"}"); + + verify(mockListener1, times(2)) + .onDecryptionFailure("my-event", "Failed to decrypt message."); } } From e694400c13717e7767b953eca7a4aa7853e8a5da Mon Sep 17 00:00:00 2001 From: Danielle Vass Date: Tue, 7 Apr 2020 14:20:52 +0100 Subject: [PATCH 12/12] Return a copy of the interestedListeners --- .../com/pusher/client/channel/impl/ChannelImpl.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/pusher/client/channel/impl/ChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/ChannelImpl.java index 157fc1c2..b6f41309 100644 --- a/src/main/java/com/pusher/client/channel/impl/ChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/ChannelImpl.java @@ -212,7 +212,15 @@ private void validateArguments(final String eventName, final SubscriptionEventLi protected Set getInterestedListeners(String event) { synchronized (lock) { - return eventNameToListenerMap.get(event); + + final Set sharedListeners = + eventNameToListenerMap.get(event); + + if (sharedListeners == null) { + return null; + } + + return new HashSet<>(sharedListeners); } } }