Author: kwall Date: Wed Sep 12 11:36:21 2012 New Revision: 1383894 URL: http://svn.apache.org/viewvc?rev=1383894&view=rev Log: QPID-4292: Java Web Management - standardise of the use of SC_FORBIDDEN and avoid ugly stack trace in logs in response to some authorisation failures
Work of Robbie Gemmell <[email protected]> and myself. Modified: qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/DefinedFileServlet.java qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/FileServlet.java qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/MessageServlet.java qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/authorization/sasl.js qpid/trunk/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/server/management/plugin/servlet/rest/VirtualHostRestTest.java qpid/trunk/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/systest/rest/acl/GroupRestACLTest.java qpid/trunk/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/systest/rest/acl/UserRestACLTest.java Modified: qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/DefinedFileServlet.java URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/DefinedFileServlet.java?rev=1383894&r1=1383893&r2=1383894&view=diff ============================================================================== --- qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/DefinedFileServlet.java (original) +++ qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/DefinedFileServlet.java Wed Sep 12 11:36:21 2012 @@ -73,7 +73,7 @@ public class DefinedFileServlet extends } else { - response.sendError(404, "unknown file: "+ _filename); + response.sendError(HttpServletResponse.SC_NOT_FOUND, "unknown file: "+ _filename); } } } Modified: qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/FileServlet.java URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/FileServlet.java?rev=1383894&r1=1383893&r2=1383894&view=diff ============================================================================== --- qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/FileServlet.java (original) +++ qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/FileServlet.java Wed Sep 12 11:36:21 2012 @@ -20,11 +20,8 @@ */ package org.apache.qpid.server.management.plugin.servlet; -import java.io.File; import java.io.IOException; import java.io.InputStream; -import java.net.URI; -import java.net.URISyntaxException; import java.net.URL; import java.util.Collections; import java.util.HashMap; @@ -101,7 +98,7 @@ public class FileServlet extends HttpSer } else { - response.sendError(404, "unknown file: "+ filename); + response.sendError(HttpServletResponse.SC_NOT_FOUND, "unknown file: "+ filename); } } Modified: qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/MessageServlet.java URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/MessageServlet.java?rev=1383894&r1=1383893&r2=1383894&view=diff ============================================================================== --- qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/MessageServlet.java (original) +++ qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/MessageServlet.java Wed Sep 12 11:36:21 2012 @@ -435,7 +435,7 @@ public class MessageServlet extends Abst } else { - response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); + response.setStatus(HttpServletResponse.SC_FORBIDDEN); } } catch(RuntimeException e) @@ -473,7 +473,7 @@ public class MessageServlet extends Abst } else { - response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); + response.setStatus(HttpServletResponse.SC_FORBIDDEN); } } Modified: qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java?rev=1383894&r1=1383893&r2=1383894&view=diff ============================================================================== --- qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java (original) +++ qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java Wed Sep 12 11:36:21 2012 @@ -19,6 +19,7 @@ package org.apache.qpid.server.managemen import java.io.BufferedWriter; import java.io.IOException; import java.io.Writer; +import java.security.AccessControlException; import java.util.*; import javax.servlet.ServletConfig; import javax.servlet.ServletException; @@ -465,10 +466,13 @@ public class RestServlet extends Abstrac private void setResponseStatus(HttpServletResponse response, RuntimeException e) throws IOException { - if (e.getCause() instanceof AMQSecurityException) + if (e instanceof AccessControlException || e.getCause() instanceof AMQSecurityException) { - LOGGER.debug("Caught AMQSecurityException", e); - response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); + if (LOGGER.isDebugEnabled()) + { + LOGGER.debug("Caught security exception, sending " + HttpServletResponse.SC_FORBIDDEN, e); + } + response.setStatus(HttpServletResponse.SC_FORBIDDEN); } else { Modified: qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java?rev=1383894&r1=1383893&r2=1383894&view=diff ============================================================================== --- qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java (original) +++ qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java Wed Sep 12 11:36:21 2012 @@ -228,7 +228,7 @@ public class SaslServlet extends Abstrac session.removeAttribute(ATTR_ID); session.removeAttribute(ATTR_SASL_SERVER); session.removeAttribute(ATTR_EXPIRY); - response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); + response.setStatus(HttpServletResponse.SC_FORBIDDEN); return; } Modified: qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/authorization/sasl.js URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/authorization/sasl.js?rev=1383894&r1=1383893&r2=1383894&view=diff ============================================================================== --- qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/authorization/sasl.js (original) +++ qpid/trunk/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/authorization/sasl.js Wed Sep 12 11:36:21 2012 @@ -71,7 +71,7 @@ var saslPlain = function saslPlain(user, }, function(error) { - if(error.status == 401) + if(error.status == 403) { alert("Authentication Failed"); } @@ -127,7 +127,7 @@ var saslCramMD5 = function saslCramMD5(u }, function(error) { - if(error.status == 401) + if(error.status == 403) { alert("Authentication Failed"); } @@ -141,7 +141,7 @@ var saslCramMD5 = function saslCramMD5(u }, function(error) { - if(error.status == 401) + if(error.status == 403) { alert("Authentication Failed"); } Modified: qpid/trunk/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/server/management/plugin/servlet/rest/VirtualHostRestTest.java URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/server/management/plugin/servlet/rest/VirtualHostRestTest.java?rev=1383894&r1=1383893&r2=1383894&view=diff ============================================================================== --- qpid/trunk/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/server/management/plugin/servlet/rest/VirtualHostRestTest.java (original) +++ qpid/trunk/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/server/management/plugin/servlet/rest/VirtualHostRestTest.java Wed Sep 12 11:36:21 2012 @@ -27,6 +27,7 @@ import java.util.List; import java.util.Map; import javax.jms.Session; +import javax.servlet.http.HttpServletResponse; import org.apache.qpid.client.AMQConnection; import org.apache.qpid.server.model.Exchange; @@ -189,7 +190,7 @@ public class VirtualHostRestTest extends { String queueName = getTestQueueName() + "-sorted"; int responseCode = tryCreateQueue(queueName, "sorted", null); - assertEquals("Unexpected response code", 409, responseCode); + assertEquals("Unexpected response code", HttpServletResponse.SC_CONFLICT, responseCode); Map<String, Object> hostDetails = getRestTestHelper().getJsonAsSingletonList("/rest/virtualhost/test"); @@ -234,7 +235,7 @@ public class VirtualHostRestTest extends { String queueName = getTestQueueName(); int responseCode = tryCreateQueue(queueName, "unsupported", null); - assertEquals("Unexpected response code", 409, responseCode); + assertEquals("Unexpected response code", HttpServletResponse.SC_CONFLICT, responseCode); Map<String, Object> hostDetails = getRestTestHelper().getJsonAsSingletonList("/rest/virtualhost/test"); Modified: qpid/trunk/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/systest/rest/acl/GroupRestACLTest.java URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/systest/rest/acl/GroupRestACLTest.java?rev=1383894&r1=1383893&r2=1383894&view=diff ============================================================================== --- qpid/trunk/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/systest/rest/acl/GroupRestACLTest.java (original) +++ qpid/trunk/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/systest/rest/acl/GroupRestACLTest.java Wed Sep 12 11:36:21 2012 @@ -107,8 +107,7 @@ public class GroupRestACLTest extends Qp getRestTestHelper().setUsernameAndPassword(DENIED_USER, DENIED_USER); - //TODO: the expected response code needs changed when we overhaul the brokers error handling - getRestTestHelper().createGroup("anotherNewGroup", FILE_GROUP_MANAGER, HttpServletResponse.SC_CONFLICT); + getRestTestHelper().createGroup("anotherNewGroup", FILE_GROUP_MANAGER, HttpServletResponse.SC_FORBIDDEN); data = getRestTestHelper().getJsonAsSingletonList("/rest/groupprovider/" + FILE_GROUP_MANAGER); getRestTestHelper().assertNumberOfGroups(data, 4); @@ -128,8 +127,7 @@ public class GroupRestACLTest extends Qp Map<String, Object> data = getRestTestHelper().getJsonAsSingletonList("/rest/groupprovider/" + FILE_GROUP_MANAGER); getRestTestHelper().assertNumberOfGroups(data, 3); - //TODO: the expected response code needs changed when we overhaul the brokers error handling - getRestTestHelper().removeGroup(OTHER_GROUP, FILE_GROUP_MANAGER, HttpServletResponse.SC_CONFLICT); + getRestTestHelper().removeGroup(OTHER_GROUP, FILE_GROUP_MANAGER, HttpServletResponse.SC_FORBIDDEN); data = getRestTestHelper().getJsonAsSingletonList("/rest/groupprovider/" + FILE_GROUP_MANAGER); getRestTestHelper().assertNumberOfGroups(data, 3); @@ -155,7 +153,7 @@ public class GroupRestACLTest extends Qp assertNumberOfGroupMembers(OTHER_GROUP, 1); - getRestTestHelper().createNewGroupMember(FILE_GROUP_MANAGER, OTHER_GROUP, "newGroupMember", HttpServletResponse.SC_CONFLICT); + getRestTestHelper().createNewGroupMember(FILE_GROUP_MANAGER, OTHER_GROUP, "newGroupMember", HttpServletResponse.SC_FORBIDDEN); assertNumberOfGroupMembers(OTHER_GROUP, 1); getRestTestHelper().setUsernameAndPassword(ALLOWED_USER, ALLOWED_USER); @@ -176,7 +174,7 @@ public class GroupRestACLTest extends Qp assertNumberOfGroupMembers(OTHER_GROUP, 1); - getRestTestHelper().removeMemberFromGroup(FILE_GROUP_MANAGER, OTHER_GROUP, OTHER_USER, HttpServletResponse.SC_CONFLICT); + getRestTestHelper().removeMemberFromGroup(FILE_GROUP_MANAGER, OTHER_GROUP, OTHER_USER, HttpServletResponse.SC_FORBIDDEN); assertNumberOfGroupMembers(OTHER_GROUP, 1); getRestTestHelper().setUsernameAndPassword(ALLOWED_USER, ALLOWED_USER); Modified: qpid/trunk/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/systest/rest/acl/UserRestACLTest.java URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/systest/rest/acl/UserRestACLTest.java?rev=1383894&r1=1383893&r2=1383894&view=diff ============================================================================== --- qpid/trunk/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/systest/rest/acl/UserRestACLTest.java (original) +++ qpid/trunk/qpid/java/broker-plugins/management-http/src/test/java/org/apache/qpid/systest/rest/acl/UserRestACLTest.java Wed Sep 12 11:36:21 2012 @@ -105,7 +105,7 @@ public class UserRestACLTest extends Qpi getRestTestHelper().setUsernameAndPassword(DENIED_USER, DENIED_USER); - getRestTestHelper().createOrUpdateUser(newUser, password, HttpServletResponse.SC_CONFLICT); + getRestTestHelper().createOrUpdateUser(newUser, password, HttpServletResponse.SC_FORBIDDEN); assertUserDoesNotExist(newUser); getRestTestHelper().setUsernameAndPassword(ALLOWED_USER, ALLOWED_USER); @@ -126,7 +126,7 @@ public class UserRestACLTest extends Qpi assertUserExists(OTHER_USER); getRestTestHelper().setUsernameAndPassword(DENIED_USER, DENIED_USER); - getRestTestHelper().removeUser(OTHER_USER, HttpServletResponse.SC_CONFLICT); + getRestTestHelper().removeUser(OTHER_USER, HttpServletResponse.SC_FORBIDDEN); assertUserExists(OTHER_USER); getRestTestHelper().setUsernameAndPassword(ALLOWED_USER, ALLOWED_USER); @@ -149,7 +149,7 @@ public class UserRestACLTest extends Qpi checkPassword(OTHER_USER, OTHER_USER, true); getRestTestHelper().setUsernameAndPassword(DENIED_USER, DENIED_USER); - getRestTestHelper().createOrUpdateUser(OTHER_USER, newPassword, HttpServletResponse.SC_CONFLICT); + getRestTestHelper().createOrUpdateUser(OTHER_USER, newPassword, HttpServletResponse.SC_FORBIDDEN); checkPassword(OTHER_USER, newPassword, false); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
