From 61dd3e62d7f8318804110bd0c0ffe678f83cbc44 Mon Sep 17 00:00:00 2001 From: Will Sewell Date: Fri, 30 Aug 2019 10:45:28 +0100 Subject: [PATCH 1/4] Add test to check user ID is stored in PresenceChannel --- .../client/channel/impl/PresenceChannelImplTest.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/pusher/client/channel/impl/PresenceChannelImplTest.java b/src/test/java/com/pusher/client/channel/impl/PresenceChannelImplTest.java index 0c47f737..177e7367 100644 --- a/src/test/java/com/pusher/client/channel/impl/PresenceChannelImplTest.java +++ b/src/test/java/com/pusher/client/channel/impl/PresenceChannelImplTest.java @@ -27,7 +27,7 @@ @RunWith(MockitoJUnitRunner.class) public class PresenceChannelImplTest extends PrivateChannelImplTest { - private static final String AUTH_RESPONSE = "\"auth\":\"a87fe72c6f36272aa4b1:f9db294eae7\",\"channel_data\":\"{\\\"user_id\\\":\\\"51169fc47abac\\\",\\\"user_info\\\":{\\\"name\\\":\\\"Phil Leggetter\\\",\\\"twitter_id\\\":\\\"@leggetter\\\"}}\""; + private static final String AUTH_RESPONSE = "\"auth\":\"a87fe72c6f36272aa4b1:f9db294eae7\",\"channel_data\":\"{\\\"user_id\\\":\\\"5116a4519575b\\\",\\\"user_info\\\":{\\\"name\\\":\\\"Phil Leggetter\\\",\\\"twitter_id\\\":\\\"@leggetter\\\"}}\""; private static final String AUTH_RESPONSE_NUMERIC_ID = "\"auth\":\"a87fe72c6f36272aa4b1:f9db294eae7\",\"channel_data\":\"{\\\"user_id\\\":51169,\\\"user_info\\\":{\\\"name\\\":\\\"Phil Leggetter\\\",\\\"twitter_id\\\":\\\"@leggetter\\\"}}\""; private static final String USER_ID = "5116a4519575b"; @@ -60,6 +60,14 @@ public void testReturnsCorrectSubscribeMessageWhenNumericId() { + AUTH_RESPONSE_NUMERIC_ID + "}}", message); } + @Test + public void testStoresCorrectUser() { + channel.toSubscribeMessage(); + channel.onMessage("pusher_internal:subscription_succeeded", + "{\"event\":\"pusher_internal:subscription_succeeded\",\"data\":\"{\\\"presence\\\":{\\\"count\\\":1,\\\"ids\\\":[\\\"5116a4519575b\\\"],\\\"hash\\\":{\\\"5116a4519575b\\\":{\\\"name\\\":\\\"Phil Leggetter\\\",\\\"twitter_id\\\":\\\"@leggetter\\\"}}}}\",\"channel\":\"presence-myChannel\"}"); + assertEquals(USER_ID, ((PresenceChannelImpl)channel).getMe().getId()); + } + @Override @Test public void testIsSubscribedMethod(){ From 9745b3dc358b2fe3f111ea17b882a8a879c8c049 Mon Sep 17 00:00:00 2001 From: Will Sewell Date: Fri, 30 Aug 2019 10:45:35 +0100 Subject: [PATCH 2/4] Forward channel_data with subscription on private channels If the auth endpoint returns channel_data, then this should be forwarded to the auth endpoint even if it is not used by private channels currently. This is useful because it allows the same auth endpoint to be used for private and presence channels. It also avoids confusing auth signature errors that occur when the channel_data is not sent (but the auth signature was computed from it). Fixes #144 #144 #144. In order to support the getMe() method on PresenceChannel, we need to store the channel_data in the PrivateChannelImpl class. This is decoded in the PresenceChannelImpl class since presence channels require the channel_data is JSON. It is decoded in toSubscribeMessage() rather than getMe() to preserve the original semantics (e.g. exceptions cannot be thrown from getMe(). --- .../channel/impl/PresenceChannelImpl.java | 32 ++----------------- .../channel/impl/PrivateChannelImpl.java | 6 ++++ .../channel/impl/PrivateChannelImplTest.java | 16 ++++++++-- 3 files changed, 22 insertions(+), 32 deletions(-) diff --git a/src/main/java/com/pusher/client/channel/impl/PresenceChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/PresenceChannelImpl.java index f34f0826..fb2738ae 100644 --- a/src/main/java/com/pusher/client/channel/impl/PresenceChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/PresenceChannelImpl.java @@ -2,7 +2,6 @@ import com.google.gson.Gson; import com.google.gson.annotations.SerializedName; -import com.pusher.client.AuthorizationFailureException; import com.pusher.client.Authorizer; import com.pusher.client.channel.ChannelEventListener; import com.pusher.client.channel.PresenceChannel; @@ -65,35 +64,10 @@ else if (event.equals(MEMBER_REMOVED_EVENT)) { } @Override - @SuppressWarnings("rawtypes") public String toSubscribeMessage() { - - final String authResponse = getAuthResponse(); - - try { - final Map authResponseMap = GSON.fromJson(authResponse, Map.class); - final String authKey = (String)authResponseMap.get("auth"); - final Object channelData = authResponseMap.get("channel_data"); - - storeMyUserId(channelData); - - final Map jsonObject = new LinkedHashMap(); - jsonObject.put("event", "pusher:subscribe"); - - final Map dataMap = new LinkedHashMap(); - dataMap.put("channel", name); - dataMap.put("auth", authKey); - dataMap.put("channel_data", channelData); - - jsonObject.put("data", dataMap); - - final String json = GSON.toJson(jsonObject); - - return json; - } - catch (final Exception e) { - throw new AuthorizationFailureException("Unable to parse response from Authorizer: " + authResponse, e); - } + String msg = super.toSubscribeMessage(); + storeMyUserId(channelData); + return msg; } @Override diff --git a/src/main/java/com/pusher/client/channel/impl/PrivateChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/PrivateChannelImpl.java index 3737c8d7..5260fdf6 100644 --- a/src/main/java/com/pusher/client/channel/impl/PrivateChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/PrivateChannelImpl.java @@ -23,6 +23,8 @@ public class PrivateChannelImpl extends ChannelImpl implements PrivateChannel { private final InternalConnection connection; private final Authorizer authorizer; + protected String channelData; + public PrivateChannelImpl(final InternalConnection connection, final String channelName, final Authorizer authorizer, final Factory factory) { super(channelName, factory); @@ -90,6 +92,7 @@ public String toSubscribeMessage() { try { final Map authResponseMap = GSON.fromJson(authResponse, Map.class); final String authKey = (String)authResponseMap.get("auth"); + channelData = (String)authResponseMap.get("channel_data"); final Map jsonObject = new LinkedHashMap(); jsonObject.put("event", "pusher:subscribe"); @@ -97,6 +100,9 @@ public String toSubscribeMessage() { final Map dataMap = new LinkedHashMap(); dataMap.put("channel", name); dataMap.put("auth", authKey); + if (channelData != null) { + dataMap.put("channel_data", channelData); + } jsonObject.put("data", dataMap); diff --git a/src/test/java/com/pusher/client/channel/impl/PrivateChannelImplTest.java b/src/test/java/com/pusher/client/channel/impl/PrivateChannelImplTest.java index f3dda768..1bf0e6e7 100644 --- a/src/test/java/com/pusher/client/channel/impl/PrivateChannelImplTest.java +++ b/src/test/java/com/pusher/client/channel/impl/PrivateChannelImplTest.java @@ -21,7 +21,8 @@ @RunWith(MockitoJUnitRunner.class) public class PrivateChannelImplTest extends ChannelImplTest { - private static final String AUTH_TOKEN = "\"auth\":\"a87fe72c6f36272aa4b1:41dce43734b18bb\""; + private static final String AUTH_RESPONSE = "\"auth\":\"a87fe72c6f36272aa4b1:41dce43734b18bb\""; + private static final String AUTH_RESPONSE_WITH_CHANNEL_DATA = "\"auth\":\"a87fe72c6f36272aa4b1:41dce43734b18bb\",\"channel_data\":\"{\\\"user_id\\\":\\\"51169fc47abac\\\"}\""; @Mock protected InternalConnection mockConnection; @@ -32,7 +33,7 @@ public class PrivateChannelImplTest extends ChannelImplTest { @Before public void setUp() { super.setUp(); - when(mockAuthorizer.authorize(eq(getChannelName()), anyString())).thenReturn("{" + AUTH_TOKEN + "}"); + when(mockAuthorizer.authorize(eq(getChannelName()), anyString())).thenReturn("{" + AUTH_RESPONSE + "}"); } @Test @@ -72,7 +73,16 @@ public void testPrivateChannelName() { @Test @Override public void testReturnsCorrectSubscribeMessage() { - assertEquals("{\"event\":\"pusher:subscribe\",\"data\":{\"channel\":\"" + getChannelName() + "\"," + AUTH_TOKEN + assertEquals("{\"event\":\"pusher:subscribe\",\"data\":{\"channel\":\"" + getChannelName() + "\"," + AUTH_RESPONSE + + "}}", channel.toSubscribeMessage()); + } + + @Test + public void testReturnsCorrectSubscribeMessageWithChannelData() { + when(mockAuthorizer.authorize(eq(getChannelName()), anyString())).thenReturn( + "{" + AUTH_RESPONSE_WITH_CHANNEL_DATA + "}"); + + assertEquals("{\"event\":\"pusher:subscribe\",\"data\":{\"channel\":\"" + getChannelName() + "\"," + AUTH_RESPONSE_WITH_CHANNEL_DATA + "}}", channel.toSubscribeMessage()); } From b14099d4cc93b0ea622d60b193ccbdb4481f639e Mon Sep 17 00:00:00 2001 From: Will Sewell Date: Tue, 3 Sep 2019 10:42:42 +0100 Subject: [PATCH 3/4] Throw AuthorizationFailureException if user_id is not in channel data Changed storeMyUserId to extractUserIdFromChannelData and moved the side effect of storing the id into toSubscribeMessage. --- .../channel/impl/PresenceChannelImpl.java | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/pusher/client/channel/impl/PresenceChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/PresenceChannelImpl.java index fb2738ae..83b8f3b4 100644 --- a/src/main/java/com/pusher/client/channel/impl/PresenceChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/PresenceChannelImpl.java @@ -1,7 +1,9 @@ package com.pusher.client.channel.impl; import com.google.gson.Gson; +import com.google.gson.JsonSyntaxException; import com.google.gson.annotations.SerializedName; +import com.pusher.client.AuthorizationFailureException; import com.pusher.client.Authorizer; import com.pusher.client.channel.ChannelEventListener; import com.pusher.client.channel.PresenceChannel; @@ -66,7 +68,7 @@ else if (event.equals(MEMBER_REMOVED_EVENT)) { @Override public String toSubscribeMessage() { String msg = super.toSubscribeMessage(); - storeMyUserId(channelData); + myUserID = extractUserIdFromChannelData((String)channelData); return msg; } @@ -161,9 +163,24 @@ private static PresenceData extractPresenceDataFrom(final String message) { } @SuppressWarnings("rawtypes") - private void storeMyUserId(final Object channelData) { - final Map channelDataMap = GSON.fromJson((String)channelData, Map.class); - myUserID = String.valueOf(channelDataMap.get("user_id")); + private String extractUserIdFromChannelData(final String channelData) { + final Map channelDataMap; + try { + channelDataMap = GSON.fromJson((String)channelData, Map.class); + } catch (final JsonSyntaxException e) { + throw new AuthorizationFailureException("Invalid response from Authorizer: unable to parse channel_data object: " + channelData, e); + } + Object maybeUserId; + try { + maybeUserId = channelDataMap.get("user_id"); + } catch (final NullPointerException e) { + throw new AuthorizationFailureException("Invalid response from Authorizer: no user_id key in channel_data object: " + channelData); + } + if (maybeUserId == null) { + throw new AuthorizationFailureException("Invalid response from Authorizer: no user_id key in channel_data object: " + channelData); + } + // user_id can be a string or an integer in the Channels websocket protocol + return String.valueOf(maybeUserId); } private class MemberData { From 1b541ec8287e9def466287f4b0abc451c3ac8397 Mon Sep 17 00:00:00 2001 From: Will Sewell Date: Fri, 6 Sep 2019 13:55:37 +0100 Subject: [PATCH 4/4] Don't cast channelData Co-Authored-By: Tom Kemp --- .../com/pusher/client/channel/impl/PresenceChannelImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/pusher/client/channel/impl/PresenceChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/PresenceChannelImpl.java index 83b8f3b4..230adc88 100644 --- a/src/main/java/com/pusher/client/channel/impl/PresenceChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/PresenceChannelImpl.java @@ -68,7 +68,7 @@ else if (event.equals(MEMBER_REMOVED_EVENT)) { @Override public String toSubscribeMessage() { String msg = super.toSubscribeMessage(); - myUserID = extractUserIdFromChannelData((String)channelData); + myUserID = extractUserIdFromChannelData(channelData); return msg; }