Check topic access when sending last will

The check is actually already done at the AMQP level, but this commit adds an "upstream" check, a more specific warning message, and a test. Fixes #114
rabbitmq · Feb 21, 2017 · ae6c077 · ae6c077
1 parent c3ca4a5
commit ae6c077
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 6 deletions.
diff --git a/src/rabbit_mqtt_processor.erl b/src/rabbit_mqtt_processor.erl
@@ -705,10 +705,17 @@ send_will(PState = #proc_state{will_msg = undefined}) ->
     PState;
 
 send_will(PState = #proc_state{will_msg = WillMsg = #mqtt_msg{retain = Retain, topic = Topic}, retainer_pid = RPid}) ->
-    amqp_pub(WillMsg, PState),
-    case Retain of
-        false -> ok;
-        true  -> hand_off_to_retainer(RPid, Topic, WillMsg)
+    case check_topic_access(Topic, write, PState) of
+        ok ->
+            amqp_pub(WillMsg, PState),
+            case Retain of
+                false -> ok;
+                true  -> hand_off_to_retainer(RPid, Topic, WillMsg)
+            end;
+        Error  ->
+            rabbit_log:warning(
+                "Could not send last will: ~p~n",
+                [Error])
     end,
     PState.
 

diff --git a/test/java_SUITE.erl b/test/java_SUITE.erl
@@ -84,7 +84,7 @@ init_per_testcase(Testcase, Config) ->
     {ok, _} = rabbit_ct_broker_helpers:rabbitmqctl(Config, 0,
         ["set_topic_permissions",  "-p", "/", "guest", "amq.topic",
             "test-topic|test-retained-topic|.*mid.*|.*topic.*",
-            "test-topic|test-retained-topic|.*mid.*|.*topic.*"]),
+            "test-topic|test-retained-topic|.*mid.*|.*topic.*|last-will"]),
     rabbit_ct_helpers:testcase_started(Config, Testcase).
 
 end_per_testcase(Testcase, Config) ->

diff --git a/test/java_SUITE_data/src/test/java/com/rabbitmq/mqtt/test/MqttTest.java b/test/java_SUITE_data/src/test/java/com/rabbitmq/mqtt/test/MqttTest.java
@@ -695,6 +695,53 @@ public Socket createSocket() throws IOException {
         }
     }
 
+    @Test public void lastWillNotSentOnRestrictedTopic() throws Exception {
+        client2.connect(conOpt);
+        // topic authorized for subscription, restricted for publishing
+        String lastWillTopic = "last-will";
+        client2.subscribe(lastWillTopic);
+        client2.setCallback(this);
+
+        final SocketFactory factory = SocketFactory.getDefault();
+        final ArrayList<Socket> sockets = new ArrayList<Socket>();
+        SocketFactory testFactory = new SocketFactory() {
+            public Socket createSocket(String s, int i) throws IOException {
+                Socket sock = factory.createSocket(s, i);
+                sockets.add(sock);
+                return sock;
+            }
+            public Socket createSocket(String s, int i, InetAddress a, int i1) throws IOException {
+                return null;
+            }
+            public Socket createSocket(InetAddress a, int i) throws IOException {
+                return null;
+            }
+            public Socket createSocket(InetAddress a, int i, InetAddress a1, int i1) throws IOException {
+                return null;
+            }
+            @Override
+            public Socket createSocket() throws IOException {
+                Socket sock = new Socket();
+                sockets.add(sock);
+                return sock;
+            }
+        };
+        conOpt.setSocketFactory(testFactory);
+        MqttTopic willTopic = client.getTopic(lastWillTopic);
+        conOpt.setWill(willTopic, payload, 0, false);
+        conOpt.setCleanSession(false);
+        client.connect(conOpt);
+
+        Assert.assertEquals(1, sockets.size());
+        expectConnectionFailure = true;
+        sockets.get(0).close();
+
+        // let some time after disconnection
+        waitForTestDelay();
+        Assert.assertEquals(0, receivedMessages.size());
+        client2.disconnect();
+    }
+
     @Test public void interopM2A() throws MqttException, IOException, InterruptedException, TimeoutException {
         setUpAmqp();
         String queue = ch.queueDeclare().getQueue();

diff --git a/test/java_SUITE_data/src/test/java/com/rabbitmq/mqtt/test/rabbit-test.sh b/test/java_SUITE_data/src/test/java/com/rabbitmq/mqtt/test/rabbit-test.sh
@@ -5,4 +5,4 @@ USER="O=client,CN=$(hostname)"
 # Test direct connections
 $CTL add_user "$USER" ''
 $CTL set_permissions -p / "$USER" ".*" ".*" ".*"
-$CTL set_topic_permissions -p / "$USER" "amq.topic" "test-topic|test-retained-topic|.*mid.*|.*topic.*"
+$CTL set_topic_permissions -p / "$USER" "amq.topic" "test-topic|test-retained-topic|.*mid.*|.*topic.*" "test-topic|test-retained-topic|.*mid.*|.*topic.*|last-will"