From 8ea734027192c5bb8e130b70091572ebaebed4d9 Mon Sep 17 00:00:00 2001 From: Marek Urbaniak <8502071+marekoid@users.noreply.github.com> Date: Thu, 2 Apr 2020 16:16:43 +0100 Subject: [PATCH 1/8] Add clearing of shared secret on disconnected Fix IndexOutOfBounds when passing all 4 args in the example app. Make the example app honour variable length of arguments array. Fix warning. Improve formatting. --- .../impl/PrivateEncryptedChannelImpl.java | 65 ++++++++++++------- .../PrivateEncryptedChannelExampleApp.java | 18 ++--- 2 files changed, 53 insertions(+), 30 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 05c46ba5..7cbbfeef 100644 --- a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java @@ -6,6 +6,9 @@ import com.pusher.client.channel.PrivateEncryptedChannel; import com.pusher.client.channel.PrivateEncryptedChannelEventListener; 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.SecretBoxOpener; import com.pusher.client.crypto.nacl.SecretBoxOpenerFactory; @@ -45,22 +48,39 @@ public void bind(final String eventName, final SubscriptionEventListener listene super.bind(eventName, listener); } - private String authenticate() { + @Override + public String toSubscribeMessage() { + String authKey = authenticate(); + // create the data part + final Map dataMap = new LinkedHashMap<>(); + dataMap.put("channel", name); + dataMap.put("auth", authKey); + + // create the wrapper part + final Map jsonObject = new LinkedHashMap<>(); + jsonObject.put("event", "pusher:subscribe"); + jsonObject.put("data", dataMap); + + return GSON.toJson(jsonObject); + } + + private String authenticate() { try { - final Map authResponseMap = GSON.fromJson(getAuthResponse(), Map.class); - final String auth = (String) authResponseMap.get("auth"); - final String sharedSecret = (String) authResponseMap.get("shared_secret"); + @SuppressWarnings("rawtypes") // anything goes in JS + final Map authResponse = GSON.fromJson(getAuthResponse(), Map.class); + + final String auth = (String) authResponse.get("auth"); + final String sharedSecret = (String) authResponse.get("shared_secret"); if (auth == null || sharedSecret == null) { throw new AuthorizationFailureException("Didn't receive all the fields expected " + "from the Authorizer, expected an auth and shared_secret."); } else { - secretBoxOpener = secretBoxOpenerFactory.create( - Base64.decode(sharedSecret)); + secretBoxOpener = secretBoxOpenerFactory.create(Base64.decode(sharedSecret)); + setListenerToClearSecretBoxOpenerOnDisconnected(); return auth; } - } catch (final AuthorizationFailureException e) { throw e; // pass this upwards } catch (final Exception e) { @@ -69,22 +89,22 @@ private String authenticate() { } } - @Override - public String toSubscribeMessage() { - - String authKey = authenticate(); - - // create the data part - final Map dataMap = new LinkedHashMap(); - dataMap.put("channel", name); - dataMap.put("auth", authKey); - - // create the wrapper part - final Map jsonObject = new LinkedHashMap(); - jsonObject.put("event", "pusher:subscribe"); - jsonObject.put("data", dataMap); + private void setListenerToClearSecretBoxOpenerOnDisconnected() { + // For not hanging on to shared secret past the Pusher.disconnect() call, + // i.e. when not necessary. Pusher.connect(...) call will trigger re-subscribe + // and hence re-authenticate which creates a new secretBoxOpener. + connection.bind(ConnectionState.DISCONNECTED, new ConnectionEventListener() { + @Override + public void onConnectionStateChange(ConnectionStateChange change) { + tearDownChannel(); // clears & nulls secretBoxOpener + connection.unbind(ConnectionState.DISCONNECTED, this); + } - return GSON.toJson(jsonObject); + @Override + public void onError(String message, String code, Exception e) { + // nop + } + }); } @Override @@ -99,6 +119,7 @@ public void updateState(ChannelState state) { private void tearDownChannel() { if (secretBoxOpener != null) { secretBoxOpener.clearKey(); + secretBoxOpener = null; } } diff --git a/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java b/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java index 340fdb57..6f19dd05 100644 --- a/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java +++ b/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java @@ -12,7 +12,7 @@ public class PrivateEncryptedChannelExampleApp implements ConnectionEventListener, PrivateEncryptedChannelEventListener { - private String apiKey = "FILL_ME_IN"; + private String apiKey = "FILL_ME_IN"; // "key" at https://dashboard.pusher.com private String channelName = "private-encrypted-channel"; private String eventName = "my-event"; private String cluster = "eu"; @@ -24,12 +24,11 @@ public static void main(final String[] args) { } private PrivateEncryptedChannelExampleApp(final String[] args) { - - if (args.length == 3) { - apiKey = args[0]; - channelName = args[1]; - eventName = args[2]; - cluster = args[3]; + switch (args.length) { + case 4: cluster = args[3]; + case 3: eventName = args[2]; + case 2: channelName = args[1]; + case 0: apiKey = args[0]; } final HttpAuthorizer authorizer = new HttpAuthorizer( @@ -45,7 +44,10 @@ private PrivateEncryptedChannelExampleApp(final String[] args) { // Keep main thread asleep while we watch for events or application will terminate while (true) { try { - Thread.sleep(1000); + Thread.sleep(5000); + pusher.disconnect(); // to put clearing of shared secret on disconnected to test + Thread.sleep(5000); + pusher.connect(this); } catch (final InterruptedException e) { e.printStackTrace(); From ddda077437ecf46a1f1afff6a37d144beeefc690 Mon Sep 17 00:00:00 2001 From: Marek Urbaniak <8502071+marekoid@users.noreply.github.com> Date: Fri, 3 Apr 2020 11:47:46 +0100 Subject: [PATCH 2/8] Make unsubscribe to remove disconnect listener Make the code more legible. That is more symmetrical when it comes to setting/removing the disconnect listener, and creating/disposing SecretBoxOpener. Make the example app exercise unsubscribe too and the two combinations (subscribed/unsubscribed) when disconnect is called. --- .../impl/PrivateEncryptedChannelImpl.java | 48 +++++++++++-------- .../PrivateEncryptedChannelExampleApp.java | 12 +++-- 2 files changed, 37 insertions(+), 23 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 7cbbfeef..8451ba50 100644 --- a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java @@ -25,6 +25,21 @@ public class PrivateEncryptedChannelImpl extends ChannelImpl implements PrivateE private SecretBoxOpenerFactory secretBoxOpenerFactory; private SecretBoxOpener secretBoxOpener; + // For not hanging on to shared secret past the Pusher.disconnect() call, + // i.e. when not necessary. Pusher.connect(...) call will trigger re-subscribe + // and hence re-authenticate which creates a new secretBoxOpener. + private ConnectionEventListener onDisconnectedListener = new ConnectionEventListener() { + @Override + public void onConnectionStateChange(ConnectionStateChange change) { + disposeSecretBoxOpener(); + } + + @Override + public void onError(String message, String code, Exception e) { + // nop + } + }; + public PrivateEncryptedChannelImpl(final InternalConnection connection, final String channelName, final Authorizer authorizer, @@ -77,8 +92,7 @@ private String authenticate() { throw new AuthorizationFailureException("Didn't receive all the fields expected " + "from the Authorizer, expected an auth and shared_secret."); } else { - secretBoxOpener = secretBoxOpenerFactory.create(Base64.decode(sharedSecret)); - setListenerToClearSecretBoxOpenerOnDisconnected(); + createSecretBoxOpener(Base64.decode(sharedSecret)); return auth; } } catch (final AuthorizationFailureException e) { @@ -89,22 +103,13 @@ private String authenticate() { } } - private void setListenerToClearSecretBoxOpenerOnDisconnected() { - // For not hanging on to shared secret past the Pusher.disconnect() call, - // i.e. when not necessary. Pusher.connect(...) call will trigger re-subscribe - // and hence re-authenticate which creates a new secretBoxOpener. - connection.bind(ConnectionState.DISCONNECTED, new ConnectionEventListener() { - @Override - public void onConnectionStateChange(ConnectionStateChange change) { - tearDownChannel(); // clears & nulls secretBoxOpener - connection.unbind(ConnectionState.DISCONNECTED, this); - } + private void createSecretBoxOpener(byte[] key) { + secretBoxOpener = secretBoxOpenerFactory.create(key); + setListenerToClearSecretBoxOpenerOnDisconnected(); + } - @Override - public void onError(String message, String code, Exception e) { - // nop - } - }); + private void setListenerToClearSecretBoxOpenerOnDisconnected() { + connection.bind(ConnectionState.DISCONNECTED, onDisconnectedListener); } @Override @@ -112,17 +117,22 @@ public void updateState(ChannelState state) { super.updateState(state); if (state == ChannelState.UNSUBSCRIBED) { - tearDownChannel(); + disposeSecretBoxOpener(); } } - private void tearDownChannel() { + private void disposeSecretBoxOpener() { if (secretBoxOpener != null) { secretBoxOpener.clearKey(); secretBoxOpener = null; + removeListenerToClearSecretBoxOpenerOnDisconnected(); } } + private void removeListenerToClearSecretBoxOpenerOnDisconnected() { + connection.unbind(ConnectionState.DISCONNECTED, onDisconnectedListener); + } + private String getAuthResponse() { final String socketId = connection.getSocketId(); return authorizer.authorize(getName(), socketId); diff --git a/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java b/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java index 6f19dd05..910ee70d 100644 --- a/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java +++ b/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java @@ -17,7 +17,7 @@ public class PrivateEncryptedChannelExampleApp implements private String eventName = "my-event"; private String cluster = "eu"; - private final PrivateEncryptedChannel channel; + private PrivateEncryptedChannel channel; public static void main(final String[] args) { new PrivateEncryptedChannelExampleApp(args); @@ -42,12 +42,16 @@ private PrivateEncryptedChannelExampleApp(final String[] args) { channel = pusher.subscribePrivateEncrypted(channelName, this, eventName); // Keep main thread asleep while we watch for events or application will terminate - while (true) { + for (int i = 0; ; i++) { try { Thread.sleep(5000); - pusher.disconnect(); // to put clearing of shared secret on disconnected to test - Thread.sleep(5000); + pusher.disconnect(); // to test clearing of shared secret pusher.connect(this); + Thread.sleep(5000); + pusher.unsubscribe(channelName); // to test clearing of shared secret + if (i % 2 == 0) { // to test disconnect on both unsubscribed/subscribed + channel = pusher.subscribePrivateEncrypted(channelName, this, eventName); + } } catch (final InterruptedException e) { e.printStackTrace(); From 8ae2cb2ee6cda4208d429f5ff7ef632a7b0940ad Mon Sep 17 00:00:00 2001 From: Marek Urbaniak <8502071+marekoid@users.noreply.github.com> Date: Fri, 3 Apr 2020 11:54:11 +0100 Subject: [PATCH 3/8] Add info about the need for tmp log for the semi-manual test --- .../client/example/PrivateEncryptedChannelExampleApp.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java b/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java index 910ee70d..45331401 100644 --- a/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java +++ b/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java @@ -45,10 +45,10 @@ private PrivateEncryptedChannelExampleApp(final String[] args) { for (int i = 0; ; i++) { try { Thread.sleep(5000); - pusher.disconnect(); // to test clearing of shared secret + pusher.disconnect(); // to test clearing of shared secret (via tmp log) pusher.connect(this); Thread.sleep(5000); - pusher.unsubscribe(channelName); // to test clearing of shared secret + pusher.unsubscribe(channelName); // to test clearing of shared secret (via tmp log) if (i % 2 == 0) { // to test disconnect on both unsubscribed/subscribed channel = pusher.subscribePrivateEncrypted(channelName, this, eventName); } From 670609e704b672dac79f91b2667b7c5cd10ce9b0 Mon Sep 17 00:00:00 2001 From: Marek Urbaniak <8502071+marekoid@users.noreply.github.com> Date: Fri, 3 Apr 2020 12:00:41 +0100 Subject: [PATCH 4/8] Fix IndexOutOfBounds for no args in example app Fix accepting a single arg. Thanks Mike for catching it: https://github.com/pusher/pusher-websocket-java/pull/247#discussion_r402922558 --- .../client/example/PrivateEncryptedChannelExampleApp.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java b/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java index 45331401..589569ad 100644 --- a/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java +++ b/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java @@ -28,7 +28,7 @@ private PrivateEncryptedChannelExampleApp(final String[] args) { case 4: cluster = args[3]; case 3: eventName = args[2]; case 2: channelName = args[1]; - case 0: apiKey = args[0]; + case 1: apiKey = args[0]; } final HttpAuthorizer authorizer = new HttpAuthorizer( From b5059d3b579da864abc23c0cd8044fe47f9dbe23 Mon Sep 17 00:00:00 2001 From: Marek Urbaniak <8502071+marekoid@users.noreply.github.com> Date: Fri, 3 Apr 2020 14:16:02 +0100 Subject: [PATCH 5/8] Make naming consistent and more clear --- .../impl/PrivateEncryptedChannelImpl.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 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 8451ba50..2d54cd85 100644 --- a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java @@ -28,7 +28,9 @@ public class PrivateEncryptedChannelImpl extends ChannelImpl implements PrivateE // For not hanging on to shared secret past the Pusher.disconnect() call, // i.e. when not necessary. Pusher.connect(...) call will trigger re-subscribe // and hence re-authenticate which creates a new secretBoxOpener. - private ConnectionEventListener onDisconnectedListener = new ConnectionEventListener() { + private ConnectionEventListener disposeSecretBoxOpenerOnDisconnectedListener = + new ConnectionEventListener() { + @Override public void onConnectionStateChange(ConnectionStateChange change) { disposeSecretBoxOpener(); @@ -105,11 +107,12 @@ private String authenticate() { private void createSecretBoxOpener(byte[] key) { secretBoxOpener = secretBoxOpenerFactory.create(key); - setListenerToClearSecretBoxOpenerOnDisconnected(); + setListenerToDisposeSecretBoxOpenerOnDisconnected(); } - private void setListenerToClearSecretBoxOpenerOnDisconnected() { - connection.bind(ConnectionState.DISCONNECTED, onDisconnectedListener); + private void setListenerToDisposeSecretBoxOpenerOnDisconnected() { + connection.bind(ConnectionState.DISCONNECTED, + disposeSecretBoxOpenerOnDisconnectedListener); } @Override @@ -125,12 +128,13 @@ private void disposeSecretBoxOpener() { if (secretBoxOpener != null) { secretBoxOpener.clearKey(); secretBoxOpener = null; - removeListenerToClearSecretBoxOpenerOnDisconnected(); + removeListenerToDisposeSecretBoxOpenerOnDisconnected(); } } - private void removeListenerToClearSecretBoxOpenerOnDisconnected() { - connection.unbind(ConnectionState.DISCONNECTED, onDisconnectedListener); + private void removeListenerToDisposeSecretBoxOpenerOnDisconnected() { + connection.unbind(ConnectionState.DISCONNECTED, + disposeSecretBoxOpenerOnDisconnectedListener); } private String getAuthResponse() { From 9c8bf51a9d48cb03b586afd535fffe2f7116ebc1 Mon Sep 17 00:00:00 2001 From: Marek Urbaniak <8502071+marekoid@users.noreply.github.com> Date: Fri, 3 Apr 2020 17:31:42 +0100 Subject: [PATCH 6/8] Add PrivateEncryptedChannelClearsKeyTest Consistently tidy up PrivateEncryptedChannelImplTest --- .../PrivateEncryptedChannelClearsKeyTest.java | 97 +++++++++++++++++++ .../impl/PrivateEncryptedChannelImplTest.java | 86 +++++----------- 2 files changed, 121 insertions(+), 62 deletions(-) create mode 100644 src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelClearsKeyTest.java diff --git a/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelClearsKeyTest.java b/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelClearsKeyTest.java new file mode 100644 index 00000000..0f936531 --- /dev/null +++ b/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelClearsKeyTest.java @@ -0,0 +1,97 @@ +package com.pusher.client.channel.impl; + +import static org.mockito.Matchers.*; +import static org.mockito.Mockito.*; + +import com.pusher.client.Authorizer; +import com.pusher.client.channel.ChannelState; +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.SecretBoxOpener; +import com.pusher.client.crypto.nacl.SecretBoxOpenerFactory; +import com.pusher.client.util.Factory; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; + +@RunWith(MockitoJUnitRunner.class) +public class PrivateEncryptedChannelClearsKeyTest { + + final String CHANNEL_NAME = "private-encrypted-unit-test-channel"; + final String AUTH_RESPONSE = "{\"auth\":\"636a81ba7e7b15725c00:3ee04892514e8a669dc5d30267221f16727596688894712cad305986e6fc0f3c\",\"shared_secret\":\"iBvNoPVYwByqSfg6anjPpEQ2j051b3rt1Vmnb+z5doo=\"}"; + + @Mock + InternalConnection mockInternalConnection; + @Mock + Authorizer mockAuthorizer; + @Mock + Factory mockFactory; + + @Mock + SecretBoxOpenerFactory mockSecretBoxOpenerFactory; + @Mock + SecretBoxOpener mockSecretBoxOpener; + + @Captor + ArgumentCaptor connectionEventListenerArgumentCaptor; + + PrivateEncryptedChannelImpl subject; + + @Before + public void setUp() { + when(mockAuthorizer.authorize(eq(CHANNEL_NAME), anyString())).thenReturn(AUTH_RESPONSE); + when(mockSecretBoxOpenerFactory.create(any())).thenReturn(mockSecretBoxOpener); + + subject = new PrivateEncryptedChannelImpl(mockInternalConnection, CHANNEL_NAME, + mockAuthorizer, mockFactory, mockSecretBoxOpenerFactory); + } + + @Test + public void secretBoxOpenerIsClearedOnUnsubscribed() { + subject.toSubscribeMessage(); + + subject.updateState(ChannelState.UNSUBSCRIBED); + + verify(mockSecretBoxOpener).clearKey(); + } + + @Test + public void secretBoxOpenerIsClearedOnDisconnected() { + doAnswer((Answer) invocation -> { + ConnectionEventListener l = (ConnectionEventListener) invocation.getArguments()[1]; + l.onConnectionStateChange(new ConnectionStateChange( + ConnectionState.DISCONNECTING, + ConnectionState.DISCONNECTED + )); + return null; + }).when(mockInternalConnection).bind(eq(ConnectionState.DISCONNECTED), any()); + subject.toSubscribeMessage(); + + verify(mockSecretBoxOpener).clearKey(); + } + + @Test + public void secretBoxOpenerIsClearedOnceOnUnsubscribedAndThenDisconnected() { + doAnswer((Answer) invocation -> { + subject.updateState(ChannelState.UNSUBSCRIBED); + + ConnectionEventListener l = (ConnectionEventListener) invocation.getArguments()[1]; + l.onConnectionStateChange(new ConnectionStateChange( + ConnectionState.DISCONNECTING, + ConnectionState.DISCONNECTED + )); + + return null; + }).when(mockInternalConnection).bind(eq(ConnectionState.DISCONNECTED), any()); + subject.toSubscribeMessage(); + + verify(mockSecretBoxOpener).clearKey(); + } +} 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 025872b5..d9e639bb 100644 --- a/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java +++ b/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java @@ -1,15 +1,17 @@ package com.pusher.client.channel.impl; +import static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + import com.pusher.client.AuthorizationFailureException; import com.pusher.client.Authorizer; import com.pusher.client.channel.ChannelEventListener; -import com.pusher.client.channel.ChannelState; 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.Factory; - import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -17,42 +19,31 @@ import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -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.verify; -import static org.mockito.Mockito.when; - @RunWith(MockitoJUnitRunner.class) public class PrivateEncryptedChannelImplTest extends ChannelImplTest { - String authorizer_valid = "{\"auth\":\"636a81ba7e7b15725c00:3ee04892514e8a669dc5d30267221f16727596688894712cad305986e6fc0f3c\",\"shared_secret\":\"iBvNoPVYwByqSfg6anjPpEQ2j051b3rt1Vmnb+z5doo=\"}"; - String authorizer_missingAuthKey = "{\"shared_secret\":\"iBvNoPVYwByqSfg6anjPpEQ2j051b3rt1Vmnb+z5doo=\"}"; - String authorizer_missingSharedSecret = "{\"auth\":\"636a81ba7e7b15725c00:3ee04892514e8a669dc5d30267221f16727596688894712cad305986e6fc0f3c\"}"; - String authorizer_malformedJson = "potatoes"; - + final String AUTH_RESPONSE = "{\"auth\":\"636a81ba7e7b15725c00:3ee04892514e8a669dc5d30267221f16727596688894712cad305986e6fc0f3c\",\"shared_secret\":\"iBvNoPVYwByqSfg6anjPpEQ2j051b3rt1Vmnb+z5doo=\"}"; + 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"; @Mock InternalConnection mockInternalConnection; @Mock Authorizer mockAuthorizer; @Mock - Factory mockFactory; - @Mock SecretBoxOpenerFactory mockSecretBoxOpenerFactory; - @Mock - SecretBoxOpener mockSecretBoxOpener; - @Override @Before public void setUp() { super.setUp(); - when(mockAuthorizer.authorize(eq(getChannelName()), anyString())).thenReturn(authorizer_valid); + when(mockAuthorizer.authorize(eq(getChannelName()), anyString())).thenReturn(AUTH_RESPONSE); + } + + protected PrivateEncryptedChannelImpl newInstance() { + return new PrivateEncryptedChannelImpl(mockInternalConnection, getChannelName(), + mockAuthorizer, factory, mockSecretBoxOpenerFactory); } @Override @@ -117,11 +108,9 @@ public void testReturnsCorrectSubscribeMessage() { @Test public void authenticationSucceedsGivenValidAuthorizer() { when(mockAuthorizer.authorize(Matchers.anyString(), Matchers.anyString())) - .thenReturn(authorizer_valid); + .thenReturn(AUTH_RESPONSE); - PrivateEncryptedChannelImpl channel = new PrivateEncryptedChannelImpl( - mockInternalConnection, getChannelName(), mockAuthorizer, mockFactory, - mockSecretBoxOpenerFactory); + PrivateEncryptedChannelImpl channel = newInstance(); channel.toSubscribeMessage(); } @@ -133,11 +122,9 @@ protected ChannelEventListener getEventListener() { @Test(expected = AuthorizationFailureException.class) public void authenticationThrowsExceptionIfNoAuthKey() { when(mockAuthorizer.authorize(Matchers.anyString(), Matchers.anyString())) - .thenReturn(authorizer_missingAuthKey); + .thenReturn(AUTH_RESPONSE_MISSING_AUTH); - PrivateEncryptedChannelImpl channel = new PrivateEncryptedChannelImpl( - mockInternalConnection, getChannelName(), mockAuthorizer, mockFactory, - mockSecretBoxOpenerFactory); + PrivateEncryptedChannelImpl channel = newInstance(); channel.toSubscribeMessage(); } @@ -145,11 +132,9 @@ mockInternalConnection, getChannelName(), mockAuthorizer, mockFactory, @Test(expected = AuthorizationFailureException.class) public void authenticationThrowsExceptionIfNoSharedSecret() { when(mockAuthorizer.authorize(Matchers.anyString(), Matchers.anyString())) - .thenReturn(authorizer_missingSharedSecret); + .thenReturn(AUTH_RESPONSE_MISSING_SHARED_SECRET); - PrivateEncryptedChannelImpl channel = new PrivateEncryptedChannelImpl( - mockInternalConnection, getChannelName(), mockAuthorizer, mockFactory, - mockSecretBoxOpenerFactory); + PrivateEncryptedChannelImpl channel = newInstance(); channel.toSubscribeMessage(); } @@ -157,33 +142,10 @@ mockInternalConnection, getChannelName(), mockAuthorizer, mockFactory, @Test(expected = AuthorizationFailureException.class) public void authenticationThrowsExceptionIfMalformedJson() { when(mockAuthorizer.authorize(Matchers.anyString(), Matchers.anyString())) - .thenReturn(authorizer_malformedJson); + .thenReturn(AUTH_RESPONSE_INVALID_JSON); - PrivateEncryptedChannelImpl channel = new PrivateEncryptedChannelImpl( - mockInternalConnection, getChannelName(), mockAuthorizer, mockFactory, - mockSecretBoxOpenerFactory); + PrivateEncryptedChannelImpl channel = newInstance(); channel.toSubscribeMessage(); } - - /* - TESTING SECRET BOX - */ - - @Test - public void secretBoxOpenerIsCleared() { - PrivateEncryptedChannelImpl channel = new PrivateEncryptedChannelImpl( - mockInternalConnection, getChannelName(), mockAuthorizer, mockFactory, - mockSecretBoxOpenerFactory); - - when(mockAuthorizer.authorize(Matchers.anyString(), Matchers.anyString())) - .thenReturn(authorizer_valid); - when(mockSecretBoxOpenerFactory.create(any())) - .thenReturn(mockSecretBoxOpener); - - channel.toSubscribeMessage(); - - channel.updateState(ChannelState.UNSUBSCRIBED); - verify(mockSecretBoxOpener).clearKey(); - } } From 95d5ca30c758f62defb5dc7ca7c687c54cdbfc31 Mon Sep 17 00:00:00 2001 From: Marek Urbaniak <8502071+marekoid@users.noreply.github.com> Date: Fri, 3 Apr 2020 17:34:37 +0100 Subject: [PATCH 7/8] Revert making example app unsubscribe/disconnect Following Danielle's feedback: https://github.com/pusher/pusher-websocket-java/pull/247/files#r402989472 --- .../example/PrivateEncryptedChannelExampleApp.java | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java b/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java index 589569ad..b883e60f 100644 --- a/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java +++ b/src/main/java/com/pusher/client/example/PrivateEncryptedChannelExampleApp.java @@ -42,16 +42,9 @@ private PrivateEncryptedChannelExampleApp(final String[] args) { channel = pusher.subscribePrivateEncrypted(channelName, this, eventName); // Keep main thread asleep while we watch for events or application will terminate - for (int i = 0; ; i++) { + while (true) { try { - Thread.sleep(5000); - pusher.disconnect(); // to test clearing of shared secret (via tmp log) - pusher.connect(this); - Thread.sleep(5000); - pusher.unsubscribe(channelName); // to test clearing of shared secret (via tmp log) - if (i % 2 == 0) { // to test disconnect on both unsubscribed/subscribed - channel = pusher.subscribePrivateEncrypted(channelName, this, eventName); - } + Thread.sleep(1000); } catch (final InterruptedException e) { e.printStackTrace(); From 0250472be8bb254b0edf286b47fbd2b086be05c7 Mon Sep 17 00:00:00 2001 From: Marek Urbaniak <8502071+marekoid@users.noreply.github.com> Date: Fri, 3 Apr 2020 17:39:31 +0100 Subject: [PATCH 8/8] Remove unused ArgumentCaptor --- .../channel/impl/PrivateEncryptedChannelClearsKeyTest.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelClearsKeyTest.java b/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelClearsKeyTest.java index 0f936531..19521a7d 100644 --- a/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelClearsKeyTest.java +++ b/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelClearsKeyTest.java @@ -15,8 +15,6 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.ArgumentCaptor; -import org.mockito.Captor; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; import org.mockito.stubbing.Answer; @@ -39,9 +37,6 @@ public class PrivateEncryptedChannelClearsKeyTest { @Mock SecretBoxOpener mockSecretBoxOpener; - @Captor - ArgumentCaptor connectionEventListenerArgumentCaptor; - PrivateEncryptedChannelImpl subject; @Before