You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by kw...@apache.org on 2012/09/12 13:36:22 UTC
svn commit: r1383894 - in
/qpid/trunk/qpid/java/broker-plugins/management-http/src:
main/java/org/apache/qpid/server/management/plugin/servlet/
main/java/org/apache/qpid/server/management/plugin/servlet/rest/
main/java/resources/js/qpid/authorization/ ...
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 <ro...@apache.org> 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: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org