You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ta...@apache.org on 2016/03/31 00:04:45 UTC

qpid-jms git commit: QPIDJMS-163 Add support for setting the authenticated User ID value from the connection into messages that are sent. Default is to not populate the user id portion of the message and can be enabled via the configuration option jms.p

Repository: qpid-jms
Updated Branches:
  refs/heads/master cb5abb046 -> 20b458567


QPIDJMS-163 Add support for setting the authenticated User ID value from
the connection into messages that are sent.  Default is to not populate
the user id portion of the message and can be enabled via the
configuration option jms.populateJMSXUserID=true

Project: http://git-wip-us.apache.org/repos/asf/qpid-jms/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-jms/commit/20b45856
Tree: http://git-wip-us.apache.org/repos/asf/qpid-jms/tree/20b45856
Diff: http://git-wip-us.apache.org/repos/asf/qpid-jms/diff/20b45856

Branch: refs/heads/master
Commit: 20b458567a7b466bda22c5be7b26f367c8a3e778
Parents: cb5abb0
Author: Timothy Bish <ta...@gmail.com>
Authored: Wed Mar 30 18:04:34 2016 -0400
Committer: Timothy Bish <ta...@gmail.com>
Committed: Wed Mar 30 18:04:34 2016 -0400

----------------------------------------------------------------------
 .../java/org/apache/qpid/jms/JmsConnection.java |  12 +
 .../apache/qpid/jms/JmsConnectionFactory.java   |  19 ++
 .../java/org/apache/qpid/jms/JmsSession.java    |  15 +
 .../message/JmsMessagePropertyIntercepter.java  |   2 +-
 .../jms/message/facade/JmsMessageFacade.java    |  21 ++
 .../apache/qpid/jms/meta/JmsConnectionInfo.java |  20 ++
 .../amqp/message/AmqpJmsMessageFacade.java      |  26 +-
 .../qpid/jms/JmsConnectionFactoryTest.java      |  13 +
 .../integration/ProducerIntegrationTest.java    | 298 +++++++++++++++++++
 .../facade/test/JmsTestMessageFacade.java       |  14 +
 .../amqp/message/AmqpJmsMessageFacadeTest.java  |  94 +++++-
 11 files changed, 527 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/20b45856/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnection.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnection.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnection.java
index a26517b..ee0bba4 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnection.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnection.java
@@ -953,6 +953,10 @@ public class JmsConnection implements AutoCloseable, Connection, TopicConnection
         connectionInfo.setUsername(username);;
     }
 
+    byte[] getEncodedUsername() {
+        return connectionInfo.getEncodedUsername();
+    }
+
     public String getPassword() {
         return connectionInfo.getPassword();
     }
@@ -1016,6 +1020,14 @@ public class JmsConnection implements AutoCloseable, Connection, TopicConnection
         this.messageIDBuilder = messageIDBuilder;
     }
 
+    public boolean isPopulateJMSXUserID() {
+        return connectionInfo.isPopulateJMSXUserID();
+    }
+
+    public void setPopulateJMSXUserID(boolean populateJMSXUserID) {
+        connectionInfo.setPopulateJMSXUserID(populateJMSXUserID);
+    }
+
     //----- Async event handlers ---------------------------------------------//
 
     @Override

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/20b45856/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnectionFactory.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnectionFactory.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnectionFactory.java
index 60b9628..bff10cc 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnectionFactory.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnectionFactory.java
@@ -70,6 +70,7 @@ public class JmsConnectionFactory extends JNDIStorable implements ConnectionFact
     private boolean localMessageExpiry = true;
     private boolean receiveLocalOnly;
     private boolean receiveNoWaitLocalOnly;
+    private boolean populateJMSXUserID;
     private String queuePrefix = null;
     private String topicPrefix = null;
     private boolean validatePropertyNames = true;
@@ -770,6 +771,24 @@ public class JmsConnectionFactory extends JNDIStorable implements ConnectionFact
         this.receiveNoWaitLocalOnly = receiveNoWaitLocalOnly;
     }
 
+    public boolean isPopulateJMSXUserID() {
+        return populateJMSXUserID;
+    }
+
+    /**
+     * Controls whether message sent from the Connection will have the JMSXUserID message
+     * property populated with the authenticated user ID of the Connection.  When false all
+     * messages sent from the Connection will not carry any value in the JMSXUserID property
+     * regardless of it being manually set on the Message to prevent a client spoofing the
+     * JMSXUserID value.
+     *
+     * @param populateJMSXUserID
+     *      true if message sent from this connection should have the JMSXUserID value populated.
+     */
+    public void setPopulateJMSXUserID(boolean populateJMSXUserID) {
+        this.populateJMSXUserID = populateJMSXUserID;
+    }
+
     //----- Static Methods ---------------------------------------------------//
 
     /**

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/20b45856/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsSession.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsSession.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsSession.java
index d99b0b2..a9ef004 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsSession.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsSession.java
@@ -653,12 +653,27 @@ public class JmsSession implements AutoCloseable, Session, QueueSession, TopicSe
             if (isJmsMessage) {
                 JmsMessage jmsMessage = (JmsMessage) original;
                 jmsMessage.getFacade().setProviderMessageIdObject(messageId);
+
+                if (connection.isPopulateJMSXUserID()) {
+                    jmsMessage.getFacade().setUserIdBytes(connection.getEncodedUsername());
+                } else {
+                    // Prevent user spoofing the user ID value.
+                    jmsMessage.getFacade().setUserId(null);
+                }
+
                 copy = jmsMessage.copy();
             } else {
                 copy = JmsMessageTransformation.transformMessage(connection, original);
                 copy.getFacade().setProviderMessageIdObject(messageId);
                 copy.setJMSDestination(destination);
 
+                if (connection.isPopulateJMSXUserID()) {
+                    copy.getFacade().setUserIdBytes(connection.getEncodedUsername());
+                } else {
+                    // Prevent user spoofing the user ID value.
+                    copy.getFacade().setUserId(null);
+                }
+
                 // If the original was a foreign message, we still need to update it
                 // with the properly encoded Message ID String, get it from the copy.
                 original.setJMSMessageID(copy.getJMSMessageID());

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/20b45856/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsMessagePropertyIntercepter.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsMessagePropertyIntercepter.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsMessagePropertyIntercepter.java
index d02e633..64d400d 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsMessagePropertyIntercepter.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsMessagePropertyIntercepter.java
@@ -567,7 +567,7 @@ public class JmsMessagePropertyIntercepter {
 
             @Override
             public void setProperty(JmsMessage message, Object value) throws JMSException {
-                if (!(value instanceof String)) {
+                if (value != null && !(value instanceof String)) {
                     throw new JMSException("Property JMSXUserID cannot be set from a " + value.getClass().getName() + ".");
                 }
                 message.getFacade().setUserId((String) value);

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/20b45856/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/facade/JmsMessageFacade.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/facade/JmsMessageFacade.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/facade/JmsMessageFacade.java
index 41ef9c9..35be150 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/facade/JmsMessageFacade.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/facade/JmsMessageFacade.java
@@ -360,6 +360,27 @@ public interface JmsMessageFacade {
     void setUserId(String userId);
 
     /**
+     * Gets the set user ID of the message in raw bytes form.  If no ID was
+     * set then this method may return null or an empty byte array.
+     *
+     * @return a byte array containing the user ID value in raw form.
+     *
+     * @throws JMSException if an error occurs while accessing the property.
+     */
+    byte[] getUserIdBytes() throws JMSException;
+
+    /**
+     * Sets the user ID of the message in raw byte form.  Setting the value
+     * as null or an empty byte array will clear any previously set value.  If the
+     * underlying protocol cannot convert or map the given byte value to it's own
+     * internal representation it should throw a JMSException indicating the error.
+     *
+     * @param userId
+     *        the byte array to use to set the message user ID.
+     */
+    void setUserIdBytes(byte[] userId);
+
+    /**
      * Gets the Group ID that this message is assigned to.
      *
      * @return the Group ID this message was sent in.

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/20b45856/qpid-jms-client/src/main/java/org/apache/qpid/jms/meta/JmsConnectionInfo.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/meta/JmsConnectionInfo.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/meta/JmsConnectionInfo.java
index 1c8fa7f..d97d33d 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/meta/JmsConnectionInfo.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/meta/JmsConnectionInfo.java
@@ -17,6 +17,7 @@
 package org.apache.qpid.jms.meta;
 
 import java.net.URI;
+import java.nio.charset.Charset;
 
 import org.apache.qpid.jms.JmsPrefetchPolicy;
 import org.apache.qpid.jms.JmsRedeliveryPolicy;
@@ -48,6 +49,7 @@ public final class JmsConnectionInfo implements JmsResource, Comparable<JmsConne
     private boolean receiveNoWaitLocalOnly;
     private boolean localMessagePriority;
     private boolean localMessageExpiry;
+    private boolean populateJMSXUserID;
     private long sendTimeout = DEFAULT_SEND_TIMEOUT;
     private long requestTimeout = DEFAULT_REQUEST_TIMEOUT;
     private long connectTimeout = DEFAULT_CONNECT_TIMEOUT;
@@ -58,6 +60,8 @@ public final class JmsConnectionInfo implements JmsResource, Comparable<JmsConne
     private JmsPrefetchPolicy prefetchPolicy = new JmsPrefetchPolicy();
     private JmsRedeliveryPolicy redeliveryPolicy = new JmsRedeliveryPolicy();
 
+    private volatile byte[] encodedUserId;
+
     public JmsConnectionInfo(JmsConnectionId connectionId) {
         if (connectionId == null) {
             throw new IllegalArgumentException("ConnectionId cannot be null");
@@ -260,6 +264,22 @@ public final class JmsConnectionInfo implements JmsResource, Comparable<JmsConne
         this.redeliveryPolicy = redeliveryPolicy.copy();
     }
 
+    public boolean isPopulateJMSXUserID() {
+        return populateJMSXUserID;
+    }
+
+    public void setPopulateJMSXUserID(boolean populateMessageUserID) {
+        this.populateJMSXUserID = populateMessageUserID;
+    }
+
+    public byte[] getEncodedUsername() {
+        if (encodedUserId == null && username != null) {
+            encodedUserId = username.getBytes(Charset.forName("UTF-8"));
+        }
+
+        return encodedUserId;
+    }
+
     @Override
     public String toString() {
         return "JmsConnectionInfo { " + getId() +

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/20b45856/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacade.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacade.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacade.java
index a337036..a739999 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacade.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacade.java
@@ -342,7 +342,7 @@ public class AmqpJmsMessageFacade implements JmsMessageFacade {
 
     @Override
     public void setProviderMessageIdObject(Object messageId) {
-            message.setMessageId(messageId);
+        message.setMessageId(messageId);
     }
 
     @Override
@@ -684,7 +684,29 @@ public class AmqpJmsMessageFacade implements JmsMessageFacade {
             bytes = userId.getBytes(UTF8);
         }
 
-        message.setUserId(bytes);
+        if (bytes == null) {
+            if (message.getProperties() != null) {
+                message.getProperties().setUserId(null);
+            }
+        } else {
+            message.setUserId(bytes);
+        }
+    }
+
+    @Override
+    public byte[] getUserIdBytes() {
+        return message.getUserId();
+    }
+
+    @Override
+    public void setUserIdBytes(byte[] userId) {
+        if (userId == null || userId.length == 0) {
+            if (message.getProperties() != null) {
+                message.getProperties().setUserId(null);
+            }
+        } else {
+            message.setUserId(userId);
+        }
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/20b45856/qpid-jms-client/src/test/java/org/apache/qpid/jms/JmsConnectionFactoryTest.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/JmsConnectionFactoryTest.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/JmsConnectionFactoryTest.java
index 679fff4..965e990 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/JmsConnectionFactoryTest.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/JmsConnectionFactoryTest.java
@@ -447,4 +447,17 @@ public class JmsConnectionFactoryTest extends QpidJmsTestCase {
             LOG.debug("Caught Ex -> ", jmse);
         }
     }
+
+    @Test(timeout = 5000)
+    public void testURIOptionPopulateJMSXUserID() throws Exception {
+        JmsConnectionFactory factory = new JmsConnectionFactory(
+            "amqp://127.0.0.1:5672?jms.populateJMSXUserID=true");
+
+        assertTrue(factory.isPopulateJMSXUserID());
+
+        factory = new JmsConnectionFactory(
+            "amqp://127.0.0.1:5672?jms.populateJMSXUserID=false");
+
+        assertFalse(factory.isPopulateJMSXUserID());
+    }
 }

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/20b45856/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/ProducerIntegrationTest.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/ProducerIntegrationTest.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/ProducerIntegrationTest.java
index a9b9a45..4d38dd6 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/ProducerIntegrationTest.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/ProducerIntegrationTest.java
@@ -31,7 +31,12 @@ import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.io.IOException;
+import java.nio.charset.Charset;
+import java.util.Collections;
 import java.util.Date;
+import java.util.Enumeration;
+import java.util.HashSet;
+import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
@@ -51,6 +56,7 @@ import org.apache.qpid.jms.JmsConnection;
 import org.apache.qpid.jms.JmsConnectionFactory;
 import org.apache.qpid.jms.JmsOperationTimedOutException;
 import org.apache.qpid.jms.JmsSendTimedOutException;
+import org.apache.qpid.jms.message.foreign.ForeignJmsMessage;
 import org.apache.qpid.jms.provider.amqp.message.AmqpMessageSupport;
 import org.apache.qpid.jms.test.QpidJmsTestCase;
 import org.apache.qpid.jms.test.Wait;
@@ -65,6 +71,7 @@ import org.apache.qpid.jms.test.testpeer.matchers.sections.MessageHeaderSectionM
 import org.apache.qpid.jms.test.testpeer.matchers.sections.MessagePropertiesSectionMatcher;
 import org.apache.qpid.jms.test.testpeer.matchers.sections.TransferPayloadCompositeMatcher;
 import org.apache.qpid.jms.test.testpeer.matchers.types.EncodedAmqpValueMatcher;
+import org.apache.qpid.proton.amqp.Binary;
 import org.apache.qpid.proton.amqp.UnsignedByte;
 import org.apache.qpid.proton.amqp.UnsignedInteger;
 import org.hamcrest.Matcher;
@@ -1293,4 +1300,295 @@ public class ProducerIntegrationTest extends QpidJmsTestCase {
             testPeer.waitForAllHandlersToComplete(1000);
         }
     }
+
+    @Test(timeout = 20000)
+    public void testUserIdSetWhenConfiguredForInclusion() throws Exception {
+        try (TestAmqpPeer testPeer = new TestAmqpPeer();) {
+
+            // Expect a PLAIN connection
+            String user = "user";
+            String pass = "qwerty123456";
+
+            testPeer.expectSaslPlainConnect(user, pass, null, null);
+            testPeer.expectBegin();
+            testPeer.expectBegin();
+            testPeer.expectSenderAttach();
+
+            JmsConnectionFactory factory = new JmsConnectionFactory(
+                "amqp://localhost:" + testPeer.getServerPort());
+            factory.setPopulateJMSXUserID(true);
+
+            Connection connection = factory.createConnection(user, pass);
+            Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+            Queue queue = session.createQueue("TestQueue");
+            MessageProducer producer = session.createProducer(queue);
+
+            Binary binaryUserId = new Binary(user.getBytes(Charset.forName("UTF-8")));
+            MessagePropertiesSectionMatcher propertiesMatcher = new MessagePropertiesSectionMatcher(true);
+            propertiesMatcher.withUserId(equalTo(binaryUserId));
+            MessageHeaderSectionMatcher headersMatcher = new MessageHeaderSectionMatcher(true);
+            MessageAnnotationsSectionMatcher msgAnnotationsMatcher = new MessageAnnotationsSectionMatcher(true);
+            TransferPayloadCompositeMatcher messageMatcher = new TransferPayloadCompositeMatcher();
+            messageMatcher.setPropertiesMatcher(propertiesMatcher);
+            messageMatcher.setHeadersMatcher(headersMatcher);
+            messageMatcher.setMessageAnnotationsMatcher(msgAnnotationsMatcher);
+
+            testPeer.expectTransfer(messageMatcher);
+
+            producer.send(session.createMessage());
+
+            testPeer.expectClose();
+            connection.close();
+
+            testPeer.waitForAllHandlersToComplete(1000);
+        }
+    }
+
+    @Test(timeout = 20000)
+    public void testUserIdNotSetWhenNotConfiguredForInclusion() throws Exception {
+        try (TestAmqpPeer testPeer = new TestAmqpPeer();) {
+
+            // Expect a PLAIN connection
+            String user = "user";
+            String pass = "qwerty123456";
+
+            testPeer.expectSaslPlainConnect(user, pass, null, null);
+            testPeer.expectBegin();
+            testPeer.expectBegin();
+            testPeer.expectSenderAttach();
+
+            JmsConnectionFactory factory = new JmsConnectionFactory(
+                "amqp://localhost:" + testPeer.getServerPort());
+            factory.setPopulateJMSXUserID(false);
+
+            Connection connection = factory.createConnection(user, pass);
+            Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+            Queue queue = session.createQueue("TestQueue");
+            MessageProducer producer = session.createProducer(queue);
+
+            MessagePropertiesSectionMatcher propertiesMatcher = new MessagePropertiesSectionMatcher(true);
+            propertiesMatcher.withUserId(nullValue());
+            MessageHeaderSectionMatcher headersMatcher = new MessageHeaderSectionMatcher(true);
+            MessageAnnotationsSectionMatcher msgAnnotationsMatcher = new MessageAnnotationsSectionMatcher(true);
+            TransferPayloadCompositeMatcher messageMatcher = new TransferPayloadCompositeMatcher();
+            messageMatcher.setPropertiesMatcher(propertiesMatcher);
+            messageMatcher.setHeadersMatcher(headersMatcher);
+            messageMatcher.setMessageAnnotationsMatcher(msgAnnotationsMatcher);
+
+            testPeer.expectTransfer(messageMatcher);
+
+            producer.send(session.createMessage());
+
+            testPeer.expectClose();
+            connection.close();
+
+            testPeer.waitForAllHandlersToComplete(1000);
+        }
+    }
+
+    @Test(timeout = 20000)
+    public void testUserIdNotSpoofedWhenConfiguredForInclusion() throws Exception {
+        try (TestAmqpPeer testPeer = new TestAmqpPeer();) {
+
+            // Expect a PLAIN connection
+            String user = "user";
+            String pass = "qwerty123456";
+
+            testPeer.expectSaslPlainConnect(user, pass, null, null);
+            testPeer.expectBegin();
+            testPeer.expectBegin();
+            testPeer.expectSenderAttach();
+
+            JmsConnectionFactory factory = new JmsConnectionFactory(
+                "amqp://localhost:" + testPeer.getServerPort());
+            factory.setPopulateJMSXUserID(true);
+
+            Connection connection = factory.createConnection(user, pass);
+            Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+            Queue queue = session.createQueue("TestQueue");
+            MessageProducer producer = session.createProducer(queue);
+
+            Binary binaryUserId = new Binary(user.getBytes(Charset.forName("UTF-8")));
+            MessagePropertiesSectionMatcher propertiesMatcher = new MessagePropertiesSectionMatcher(true);
+            propertiesMatcher.withUserId(equalTo(binaryUserId));
+            MessageHeaderSectionMatcher headersMatcher = new MessageHeaderSectionMatcher(true);
+            MessageAnnotationsSectionMatcher msgAnnotationsMatcher = new MessageAnnotationsSectionMatcher(true);
+            TransferPayloadCompositeMatcher messageMatcher = new TransferPayloadCompositeMatcher();
+            messageMatcher.setPropertiesMatcher(propertiesMatcher);
+            messageMatcher.setHeadersMatcher(headersMatcher);
+            messageMatcher.setMessageAnnotationsMatcher(msgAnnotationsMatcher);
+
+            testPeer.expectTransfer(messageMatcher);
+
+            Message message = session.createMessage();
+            message.setStringProperty("JMSXUserID", "spoofed");
+
+            producer.send(message);
+
+            testPeer.expectClose();
+            connection.close();
+
+            testPeer.waitForAllHandlersToComplete(1000);
+        }
+    }
+
+    @Test(timeout = 20000)
+    public void testUserIdNotSpoofedWhenNotConfiguredForInclusion() throws Exception {
+        try (TestAmqpPeer testPeer = new TestAmqpPeer();) {
+
+            // Expect a PLAIN connection
+            String user = "user";
+            String pass = "qwerty123456";
+
+            testPeer.expectSaslPlainConnect(user, pass, null, null);
+            testPeer.expectBegin();
+            testPeer.expectBegin();
+            testPeer.expectSenderAttach();
+
+            JmsConnectionFactory factory = new JmsConnectionFactory(
+                "amqp://localhost:" + testPeer.getServerPort());
+            factory.setPopulateJMSXUserID(false);
+
+            Connection connection = factory.createConnection(user, pass);
+            Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+            Queue queue = session.createQueue("TestQueue");
+            MessageProducer producer = session.createProducer(queue);
+
+            MessagePropertiesSectionMatcher propertiesMatcher = new MessagePropertiesSectionMatcher(true);
+            propertiesMatcher.withUserId(nullValue());
+            MessageHeaderSectionMatcher headersMatcher = new MessageHeaderSectionMatcher(true);
+            MessageAnnotationsSectionMatcher msgAnnotationsMatcher = new MessageAnnotationsSectionMatcher(true);
+            TransferPayloadCompositeMatcher messageMatcher = new TransferPayloadCompositeMatcher();
+            messageMatcher.setPropertiesMatcher(propertiesMatcher);
+            messageMatcher.setHeadersMatcher(headersMatcher);
+            messageMatcher.setMessageAnnotationsMatcher(msgAnnotationsMatcher);
+
+            testPeer.expectTransfer(messageMatcher);
+
+            Message message = session.createMessage();
+            message.setStringProperty("JMSXUserID", "spoofed");
+
+            producer.send(message);
+
+            testPeer.expectClose();
+            connection.close();
+
+            testPeer.waitForAllHandlersToComplete(1000);
+        }
+    }
+
+    private class CustomForeignMessage extends ForeignJmsMessage {
+
+        @Override
+        public Enumeration<?> getPropertyNames() throws JMSException {
+            Enumeration<?> properties = super.getPropertyNames();
+
+            Set<Object> names = new HashSet<Object>();
+            while (properties.hasMoreElements()) {
+                names.add(properties.nextElement());
+            }
+
+            names.add("JMSXUserID");
+
+            return Collections.enumeration(names);
+        }
+
+        @Override
+        public Object getObjectProperty(String name) throws JMSException {
+            if (name.equals("JMSXUserID")) {
+                return "spoofed";
+            }
+
+            return message.getObjectProperty(name);
+        }
+    }
+
+    @Test(timeout = 20000)
+    public void testUserIdNotSpoofedWhenConfiguredForInclusionWithForgeinMessage() throws Exception {
+        try (TestAmqpPeer testPeer = new TestAmqpPeer();) {
+
+            // Expect a PLAIN connection
+            String user = "user";
+            String pass = "qwerty123456";
+
+            testPeer.expectSaslPlainConnect(user, pass, null, null);
+            testPeer.expectBegin();
+            testPeer.expectBegin();
+            testPeer.expectSenderAttach();
+
+            JmsConnectionFactory factory = new JmsConnectionFactory(
+                "amqp://localhost:" + testPeer.getServerPort());
+            factory.setPopulateJMSXUserID(true);
+
+            Connection connection = factory.createConnection(user, pass);
+            Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+            Queue queue = session.createQueue("TestQueue");
+            MessageProducer producer = session.createProducer(queue);
+
+            Binary binaryUserId = new Binary(user.getBytes(Charset.forName("UTF-8")));
+            MessagePropertiesSectionMatcher propertiesMatcher = new MessagePropertiesSectionMatcher(true);
+            propertiesMatcher.withUserId(equalTo(binaryUserId));
+            MessageHeaderSectionMatcher headersMatcher = new MessageHeaderSectionMatcher(true);
+            MessageAnnotationsSectionMatcher msgAnnotationsMatcher = new MessageAnnotationsSectionMatcher(true);
+            TransferPayloadCompositeMatcher messageMatcher = new TransferPayloadCompositeMatcher();
+            messageMatcher.setPropertiesMatcher(propertiesMatcher);
+            messageMatcher.setHeadersMatcher(headersMatcher);
+            messageMatcher.setMessageAnnotationsMatcher(msgAnnotationsMatcher);
+
+            testPeer.expectTransfer(messageMatcher);
+
+            Message message = new CustomForeignMessage();
+            message.setStringProperty("JMSXUserID", "spoofed");
+
+            producer.send(message);
+
+            testPeer.expectClose();
+            connection.close();
+
+            testPeer.waitForAllHandlersToComplete(1000);
+        }
+    }
+
+    @Test(timeout = 20000)
+    public void testUserIdNotSpoofedWhenNotConfiguredForInclusionWithForeignMessage() throws Exception {
+        try (TestAmqpPeer testPeer = new TestAmqpPeer();) {
+
+            // Expect a PLAIN connection
+            String user = "user";
+            String pass = "qwerty123456";
+
+            testPeer.expectSaslPlainConnect(user, pass, null, null);
+            testPeer.expectBegin();
+            testPeer.expectBegin();
+            testPeer.expectSenderAttach();
+
+            JmsConnectionFactory factory = new JmsConnectionFactory(
+                "amqp://localhost:" + testPeer.getServerPort());
+            factory.setPopulateJMSXUserID(false);
+
+            Connection connection = factory.createConnection(user, pass);
+            Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+            Queue queue = session.createQueue("TestQueue");
+            MessageProducer producer = session.createProducer(queue);
+
+            MessagePropertiesSectionMatcher propertiesMatcher = new MessagePropertiesSectionMatcher(true);
+            propertiesMatcher.withUserId(nullValue());
+            MessageHeaderSectionMatcher headersMatcher = new MessageHeaderSectionMatcher(true);
+            MessageAnnotationsSectionMatcher msgAnnotationsMatcher = new MessageAnnotationsSectionMatcher(true);
+            TransferPayloadCompositeMatcher messageMatcher = new TransferPayloadCompositeMatcher();
+            messageMatcher.setPropertiesMatcher(propertiesMatcher);
+            messageMatcher.setHeadersMatcher(headersMatcher);
+            messageMatcher.setMessageAnnotationsMatcher(msgAnnotationsMatcher);
+
+            testPeer.expectTransfer(messageMatcher);
+
+            Message message = new CustomForeignMessage();
+            producer.send(message);
+
+            testPeer.expectClose();
+            connection.close();
+
+            testPeer.waitForAllHandlersToComplete(1000);
+        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/20b45856/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/test/JmsTestMessageFacade.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/test/JmsTestMessageFacade.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/test/JmsTestMessageFacade.java
index 72d1656..3a6bf93 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/test/JmsTestMessageFacade.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/test/JmsTestMessageFacade.java
@@ -313,6 +313,20 @@ public class JmsTestMessageFacade implements JmsMessageFacade {
     }
 
     @Override
+    public byte[] getUserIdBytes() throws JMSException {
+        return userId != null ? userId.getBytes(Charset.forName("UTF-8")) : null;
+    }
+
+    @Override
+    public void setUserIdBytes(byte[] userId) {
+        if (userId != null) {
+            this.userId = new String(userId, Charset.forName("UTF-8"));
+        } else {
+            this.userId = null;
+        }
+    }
+
+    @Override
     public String getGroupId() {
         return this.groupId;
     }

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/20b45856/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacadeTest.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacadeTest.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacadeTest.java
index 97e8b16..597d01f 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacadeTest.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacadeTest.java
@@ -558,7 +558,7 @@ public class AmqpJmsMessageFacadeTest extends AmqpJmsMessageTypesTestCase  {
     }
 
     /**
-     * Check that setting UserId null on the message causes any existing value to be cleared
+     * Check that setting GroupId null on the message causes any existing value to be cleared
      *
      * @throws Exception if an error occurs during the test.
      */
@@ -754,8 +754,6 @@ public class AmqpJmsMessageFacadeTest extends AmqpJmsMessageTypesTestCase  {
         AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
 
         amqpMessageFacade.setGroupSequence(5);
-
-        // TODO
         amqpMessageFacade.setGroupSequence(0);
 
         // assertNull("underlying message should have no groupSequence field value", amqpMessageFacade.getAmqpMessage().getProperties().getGroupSequence());
@@ -765,7 +763,7 @@ public class AmqpJmsMessageFacadeTest extends AmqpJmsMessageTypesTestCase  {
     @Test
     public void testClearGroupSequenceOnMessageWithoutExistingGroupSequence() {
         AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
-        // TODO
+
         amqpMessageFacade.setGroupSequence(0);
 
         assertNull("underlying message should still have no properties setion", amqpMessageFacade.getAmqpMessage().getProperties());
@@ -1430,6 +1428,94 @@ public class AmqpJmsMessageFacadeTest extends AmqpJmsMessageTypesTestCase  {
         assertNull("userid not as expected", amqpMessageFacade.getUserId());
     }
 
+    @Test
+    public void testClearUserIdWithNoExistingProperties() {
+        AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
+
+        amqpMessageFacade.setUserId(null);
+
+        assertNull("underlying message should still have no properties setion", amqpMessageFacade.getAmqpMessage().getProperties());
+        assertEquals("UserId should be null", null, amqpMessageFacade.getUserId());
+    }
+
+    // --- user-id-bytes field  ---
+
+    @Test
+    public void testGetUserIdBytesIsNullForNewMessage() {
+        AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
+
+        assertNull("expected userid bytes to be null on new message", amqpMessageFacade.getUserIdBytes());
+    }
+
+    @Test
+    public void testGetUserIdBytesOnReceievedMessage() throws Exception {
+        String userIdString = "testValue";
+        byte[] bytes = userIdString.getBytes("UTF-8");
+
+        Message message = Proton.message();
+
+        message.setUserId(bytes);
+
+        Properties props = new Properties();
+        props.setUserId(new Binary(bytes));
+        message.setProperties(props);
+
+        AmqpJmsMessageFacade amqpMessageFacade = createReceivedMessageFacade(createMockAmqpConsumer(), message);
+
+        assertNotNull("Expected a userid on received message", amqpMessageFacade.getUserIdBytes());
+        assertArrayEquals("Incorrect userid bytes value received", bytes, amqpMessageFacade.getUserIdBytes());
+    }
+
+    /**
+     * Check that setting UserId on the message causes creation of the underlying properties
+     * section with the expected value. New messages lack the properties section section,
+     * as tested by {@link #testNewMessageHasNoUnderlyingPropertiesSection()}.
+     *
+     * @throws Exception if an error occurs during the test.
+     */
+    @Test
+    public void testSetUserIdBytesOnNewMessage() throws Exception {
+        String userIdString = "testValue";
+        byte[] bytes = userIdString.getBytes("UTF-8");
+        AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
+
+        amqpMessageFacade.setUserIdBytes(bytes);
+
+        assertNotNull("properties section was not created", amqpMessageFacade.getAmqpMessage().getProperties());
+        assertTrue("bytes were not set as expected for userid", Arrays.equals(bytes, amqpMessageFacade.getAmqpMessage().getProperties().getUserId().getArray()));
+        assertArrayEquals("userid bytes not as expected", bytes, amqpMessageFacade.getUserIdBytes());
+    }
+
+    /**
+     * Check that setting UserId null on the message causes any existing value to be cleared
+     *
+     * @throws Exception if an error occurs during the test.
+     */
+    @Test
+    public void testSetUserIdBytesNullOnMessageWithExistingUserId() throws Exception {
+        String userIdString = "testValue";
+        byte[] bytes = userIdString.getBytes("UTF-8");
+
+        AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
+
+        amqpMessageFacade.setUserIdBytes(bytes);
+        amqpMessageFacade.setUserId(null);
+
+        assertNotNull("properties section was not created", amqpMessageFacade.getAmqpMessage().getProperties());
+        assertNull("bytes were not cleared as expected for userid", amqpMessageFacade.getAmqpMessage().getProperties().getUserId());
+        assertNull("userid bytes not as expected", amqpMessageFacade.getUserIdBytes());
+    }
+
+    @Test
+    public void testClearUserIdBytesWithNoExistingProperties() {
+        AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
+
+        amqpMessageFacade.setUserIdBytes(null);
+
+        assertNull("underlying message should still have no properties setion", amqpMessageFacade.getAmqpMessage().getProperties());
+        assertEquals("UserId should be null", null, amqpMessageFacade.getUserIdBytes());
+    }
+
     // ====== AMQP Message Annotations =======
     // =======================================
 


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org