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 <[email protected]> Authored: Fri May 19 15:30:34 2017 +0100 Committer: Alex Rudyy <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
