You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by or...@apache.org on 2017/05/19 15:09:11 UTC

qpid-broker-j git commit: QPID-7751: SASL REST authentication should tolerate session invalidation from concurrent SASL negotionations performed on the same session

Repository: qpid-broker-j
Updated Branches:
  refs/heads/master 9b5ee9aec -> 075612a21


QPID-7751: SASL REST authentication should tolerate session invalidation from concurrent SASL negotionations performed on the same session


Project: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/commit/075612a2
Tree: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/tree/075612a2
Diff: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/diff/075612a2

Branch: refs/heads/master
Commit: 075612a2148e690af2e8093fadc9984ef00142df
Parents: 9b5ee9a
Author: Alex Rudyy <or...@apache.org>
Authored: Fri May 19 15:30:34 2017 +0100
Committer: Alex Rudyy <or...@apache.org>
Committed: Fri May 19 15:30:34 2017 +0100

----------------------------------------------------------------------
 .../management/plugin/HttpManagementUtil.java   | 80 ++++++++++++++++++--
 .../plugin/SessionInvalidatedException.java     | 25 ++++++
 .../plugin/servlet/rest/SaslServlet.java        | 72 +++++++++---------
 .../src/main/java/resources/login.html          | 22 +++++-
 4 files changed, 152 insertions(+), 47 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/075612a2/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagementUtil.java
----------------------------------------------------------------------
diff --git a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagementUtil.java b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagementUtil.java
index 8c79a7e..c25b190 100644
--- a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagementUtil.java
+++ b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagementUtil.java
@@ -100,7 +100,18 @@ public class HttpManagementUtil
     public static Subject getAuthorisedSubject(HttpServletRequest request)
     {
         HttpSession session = request.getSession(false);
-        return (session == null ? null : (Subject) session.getAttribute(getRequestSpecificAttributeName(ATTR_SUBJECT,request)));
+        if (session == null)
+        {
+            return null;
+        }
+        try
+        {
+            return  (Subject)session.getAttribute(getRequestSpecificAttributeName(ATTR_SUBJECT, request));
+        }
+        catch (IllegalStateException e)
+        {
+            return null;
+        }
     }
 
     public static Subject createServletConnectionSubject(final HttpServletRequest request, Subject original)
@@ -130,11 +141,11 @@ public class HttpManagementUtil
     public static void saveAuthorisedSubject(HttpServletRequest request, Subject subject)
     {
         HttpSession session = request.getSession();
-        session.setAttribute(getRequestSpecificAttributeName(ATTR_SUBJECT, request), subject);
-
-        // Cause the user logon to be logged.
-        session.setAttribute(getRequestSpecificAttributeName(ATTR_LOGIN_LOGOUT_REPORTER, request),
-                             new LoginLogoutReporter(subject, getBroker(session.getServletContext())));
+        setSessionAttribute(ATTR_SUBJECT, subject, session, request);
+        setSessionAttribute(ATTR_LOGIN_LOGOUT_REPORTER,
+                            new LoginLogoutReporter(subject, getBroker(session.getServletContext())),
+                            session,
+                            request);
     }
 
     public static Subject tryToAuthenticate(HttpServletRequest request, HttpManagementConfiguration managementConfig)
@@ -246,4 +257,61 @@ public class HttpManagementUtil
         }
         return null;
     }
+
+    public static void invalidateSession(HttpSession session)
+    {
+        try
+        {
+            session.invalidate();
+        }
+        catch (IllegalStateException e)
+        {
+            // session was already invalidated
+        }
+    }
+
+    public static Object getSessionAttribute(String attributeName,
+                                             HttpSession session,
+                                             HttpServletRequest request)
+    {
+        String requestSpecificAttributeName = getRequestSpecificAttributeName(attributeName, request);
+        try
+        {
+            return session.getAttribute(requestSpecificAttributeName);
+        }
+        catch (IllegalStateException e)
+        {
+            throw new SessionInvalidatedException();
+        }
+    }
+
+    public static void setSessionAttribute(String attributeName,
+                                           Object attributeValue, HttpSession session,
+                                           HttpServletRequest request)
+    {
+        String requestSpecificAttributeName = getRequestSpecificAttributeName(attributeName, request);
+        try
+        {
+            session.setAttribute(requestSpecificAttributeName, attributeValue);
+        }
+        catch (IllegalStateException e)
+        {
+            throw new SessionInvalidatedException();
+        }
+    }
+
+    public static void removeAttribute(final String attributeName,
+                                       final HttpSession session,
+                                       final HttpServletRequest request)
+    {
+        String requestSpecificAttributeName = getRequestSpecificAttributeName(attributeName, request);
+        try
+        {
+            session.removeAttribute(requestSpecificAttributeName);
+        }
+        catch (IllegalStateException e)
+        {
+            // session was invalidated
+        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/075612a2/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/SessionInvalidatedException.java
----------------------------------------------------------------------
diff --git a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/SessionInvalidatedException.java b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/SessionInvalidatedException.java
new file mode 100644
index 0000000..60c6053
--- /dev/null
+++ b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/SessionInvalidatedException.java
@@ -0,0 +1,25 @@
+package org.apache.qpid.server.management.plugin;
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+public class SessionInvalidatedException extends RuntimeException
+{
+}

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/075612a2/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java
----------------------------------------------------------------------
diff --git a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java
index faff6e5..5609669 100644
--- a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java
+++ b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java
@@ -21,7 +21,6 @@
 package org.apache.qpid.server.management.plugin.servlet.rest;
 
 import java.io.IOException;
-import java.security.AccessControlContext;
 import java.security.AccessController;
 import java.security.Principal;
 import java.security.SecureRandom;
@@ -42,6 +41,7 @@ import org.slf4j.LoggerFactory;
 
 import org.apache.qpid.server.management.plugin.HttpManagementConfiguration;
 import org.apache.qpid.server.management.plugin.HttpManagementUtil;
+import org.apache.qpid.server.management.plugin.SessionInvalidatedException;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.security.SubjectCreator;
@@ -105,15 +105,15 @@ public class SaslServlet extends AbstractServlet
     private Random getRandom(final HttpServletRequest request)
     {
         HttpSession session = request.getSession();
-        Random rand = (Random) session.getAttribute(HttpManagementUtil.getRequestSpecificAttributeName(ATTR_RANDOM,
-                                                                                                       request));
+        Random rand = (Random) HttpManagementUtil.getSessionAttribute(ATTR_RANDOM, session, request);
+
         if(rand == null)
         {
             synchronized (SECURE_RANDOM)
             {
                 rand = new Random(SECURE_RANDOM.nextLong());
             }
-            session.setAttribute(HttpManagementUtil.getRequestSpecificAttributeName(ATTR_RANDOM, request), rand);
+            HttpManagementUtil.setSessionAttribute(ATTR_RANDOM, rand, session, request);
         }
         return rand;
     }
@@ -135,13 +135,14 @@ public class SaslServlet extends AbstractServlet
 
             SubjectCreator subjectCreator = getSubjectCreator(request);
 
+            SaslNegotiator saslNegotiator = null;
             if(mechanism != null)
             {
                 if(id == null && subjectCreator.getMechanisms().contains(mechanism))
                 {
                     LOGGER.debug("Creating SaslServer for mechanism: {}", mechanism);
 
-                    SaslNegotiator saslNegotiator = subjectCreator.createSaslNegotiator(mechanism, new SaslSettings()
+                    saslNegotiator = subjectCreator.createSaslNegotiator(mechanism, new SaslSettings()
                     {
                         @Override
                         public String getLocalFQDN()
@@ -155,46 +156,41 @@ public class SaslServlet extends AbstractServlet
                             return null/*TODO*/;
                         }
                     });
-                    evaluateSaslResponse(request, response, session, saslResponse, saslNegotiator, subjectCreator);
-                }
-                else
-                {
-                    cleanup(request, session);
-                    response.setStatus(HttpServletResponse.SC_EXPECTATION_FAILED);
                 }
             }
             else
             {
                 if(id != null)
                 {
-                    if(id.equals(session.getAttribute(HttpManagementUtil.getRequestSpecificAttributeName(ATTR_ID,
-                                                                                                         request))) && System.currentTimeMillis() < (Long) session.getAttribute(
-                            HttpManagementUtil.getRequestSpecificAttributeName(ATTR_EXPIRY, request)))
-                    {
-                        SaslNegotiator saslNegotiator =
-                                (SaslNegotiator) session.getAttribute(HttpManagementUtil.getRequestSpecificAttributeName(
-                                        ATTR_SASL_NEGOTIATOR,
-                                        request));
-                        evaluateSaslResponse(request, response, session, saslResponse, saslNegotiator, subjectCreator);
-                    }
-                    else
+                    if (id.equals(HttpManagementUtil.getSessionAttribute(ATTR_ID, session, request))
+                        && System.currentTimeMillis() < (Long) HttpManagementUtil.getSessionAttribute(ATTR_EXPIRY, session, request))
                     {
-                        cleanup(request, session);
-                        response.setStatus(HttpServletResponse.SC_EXPECTATION_FAILED);
+                        saslNegotiator = (SaslNegotiator) HttpManagementUtil.getSessionAttribute(ATTR_SASL_NEGOTIATOR,
+                                                                                                 session,
+                                                                                                 request);
                     }
                 }
-                else
-                {
-                    cleanup(request, session);
-                    response.setStatus(HttpServletResponse.SC_EXPECTATION_FAILED);
-                }
             }
+
+            if (saslNegotiator != null)
+            {
+                evaluateSaslResponse(request, response, session, saslResponse, saslNegotiator, subjectCreator);
+            }
+            else
+            {
+                cleanup(request, session);
+                response.setStatus(HttpServletResponse.SC_EXPECTATION_FAILED);
+            }
+        }
+        catch (SessionInvalidatedException e)
+        {
+            response.setStatus(HttpServletResponse.SC_PRECONDITION_FAILED);
         }
         finally
         {
             if (response.getStatus() != HttpServletResponse.SC_OK)
             {
-                session.invalidate();
+                HttpManagementUtil.invalidateSession(session);
             }
         }
     }
@@ -202,14 +198,14 @@ public class SaslServlet extends AbstractServlet
     private void cleanup(final HttpServletRequest request, final HttpSession session)
     {
         final SaslNegotiator negotiator =
-                (SaslNegotiator) session.getAttribute(HttpManagementUtil.getRequestSpecificAttributeName(ATTR_SASL_NEGOTIATOR, request));
+                (SaslNegotiator) HttpManagementUtil.getSessionAttribute(ATTR_SASL_NEGOTIATOR, session, request);
         if (negotiator != null)
         {
             negotiator.dispose();
         }
-        session.removeAttribute(HttpManagementUtil.getRequestSpecificAttributeName(ATTR_ID, request));
-        session.removeAttribute(HttpManagementUtil.getRequestSpecificAttributeName(ATTR_SASL_NEGOTIATOR, request));
-        session.removeAttribute(HttpManagementUtil.getRequestSpecificAttributeName(ATTR_EXPIRY, request));
+        HttpManagementUtil.removeAttribute(ATTR_ID, session, request);
+        HttpManagementUtil.removeAttribute(ATTR_SASL_NEGOTIATOR, session, request);
+        HttpManagementUtil.removeAttribute(ATTR_EXPIRY, session, request);
     }
 
     private void checkSaslAuthEnabled(HttpServletRequest request)
@@ -274,9 +270,11 @@ public class SaslServlet extends AbstractServlet
         {
             Random rand = getRandom(request);
             String id = String.valueOf(rand.nextLong());
-            session.setAttribute(HttpManagementUtil.getRequestSpecificAttributeName(ATTR_ID, request), id);
-            session.setAttribute(HttpManagementUtil.getRequestSpecificAttributeName(ATTR_SASL_NEGOTIATOR, request), saslNegotiator);
-            session.setAttribute(HttpManagementUtil.getRequestSpecificAttributeName(ATTR_EXPIRY, request), System.currentTimeMillis() + SASL_EXCHANGE_EXPIRY);
+            HttpManagementUtil.setSessionAttribute(ATTR_ID, id, session, request);
+            HttpManagementUtil.setSessionAttribute(ATTR_SASL_NEGOTIATOR, saslNegotiator, session, request);
+            HttpManagementUtil.setSessionAttribute(ATTR_EXPIRY,
+                                                   System.currentTimeMillis() + SASL_EXCHANGE_EXPIRY,
+                                                   session, request);
 
             outputObject.put("id", id);
             outputObject.put("challenge", DatatypeConverter.printBase64Binary(challenge));

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/075612a2/broker-plugins/management-http/src/main/java/resources/login.html
----------------------------------------------------------------------
diff --git a/broker-plugins/management-http/src/main/java/resources/login.html b/broker-plugins/management-http/src/main/java/resources/login.html
index 8fb7e3e..53d30e1 100644
--- a/broker-plugins/management-http/src/main/java/resources/login.html
+++ b/broker-plugins/management-http/src/main/java/resources/login.html
@@ -93,14 +93,18 @@
             <div data-dojo-type="dijit.form.Form" method="POST" id="loginForm">
                 <script type="dojo/on" data-dojo-event="submit" data-dojo-args="e">
                     e.preventDefault()
-                    if (this.validate())
+                    var loginButton = dijit.byId('loginButton')
+                    if (loginButton.get("login") === false && this.validate())
                     {
                         var that = this;
+                        loginButton.set("login", true);
+                        loginButton.set("disabled", true);
                         require(["qpid/management/Management", "dojo/json"],
                                 function (Management, json)
                                 {
                                     var redirectIfAuthenticated = function redirectIfAuthenticated(data)
                                     {
+                                        loginButton.set("login", false);
                                         if (data.user)
                                         {
 
@@ -113,6 +117,7 @@
                                     };
                                     var errorHandler = function errorHandler(error)
                                     {
+                                        loginButton.set("login", false);
                                         if (error.response)
                                         {
                                             if (error.response.status == 401)
@@ -144,12 +149,21 @@
                 <div data-dojo-type="dijit.TitlePane" data-dojo-props="title:'Login', toggleable: false" >
                     <div class="dijitDialogPaneContentArea">
                         <div data-dojo-type="dojox.layout.TableContainer" data-dojo-props="cols:1,labelWidth:'100',showLabels:true,orientation:'horiz',customClass:'formLabel'">
-                            <div data-dojo-type="dijit.form.ValidationTextBox" id="username" name="username" data-dojo-props="label:'User name:',required:true, intermediateChanges:true"></div>
-                            <div data-dojo-type="dijit.form.ValidationTextBox" type="password" id="password" name="password" data-dojo-props="label:'Password:',required:true, intermediateChanges:true"></div>
+                            <div data-dojo-type="dijit.form.ValidationTextBox" id="username" name="username"
+                                 data-dojo-props="label:'User name:',required:true, intermediateChanges:true,
+                                                 onChange:function(){dijit.byId('loginButton').set('disabled',
+                                                                    dijit.byId('loginButton').get('login') === true
+                                                                     || !this.get('value')
+                                                                     || !dijit.byId('password').get('value'));}"></div>
+                            <div data-dojo-type="dijit.form.ValidationTextBox" type="password" id="password" name="password"
+                                 data-dojo-props="label:'Password:',required:true, intermediateChanges:true,
+                             onChange:function(){dijit.byId('loginButton').set('disabled',
+                                                dijit.byId('loginButton').get('login') === true
+                                                || !this.get('value') || !dijit.byId('username').get('value'));}"></div>
                         </div>
                     </div>
                     <div class="dijitDialogPaneActionBar qpidDialogPaneActionBar">
-                         <button data-dojo-type="dijit.form.Button" type="submit" id="loginButton">Login</button>
+                         <button data-dojo-type="dijit.form.Button" type="submit" id="loginButton" data-dojo-props="disabled:true,login:false">Login</button>
                     </div>
                 </div>
             </div>


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