Apache
/
druid
mirror of https://github.com/apache/druid.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix PreResponseAuthorizationCheckFilter HTTP error masking (#4900) 

* Fix PreResponseAuthorizationCheckFilter HTTP error masking

* Add remote addr and host to missing auth check log message
This commit is contained in:
Jonathan Wei 2017-10-03 14:58:57 -07:00 committed by Himanshu
parent bba96f59f8
commit 07aa405a6f
2 changed files with 31 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
server/src/main/java/io/druid/server/security/PreResponseAuthorizationCheckFilter.java
View File

@ -84,7 +84,7 @@ public class PreResponseAuthorizationCheckFilter implements Filter
filterChain.doFilter(servletRequest, servletResponse); filterChain.doFilter(servletRequest, servletResponse);
Boolean authInfoChecked = (Boolean) servletRequest.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED); Boolean authInfoChecked = (Boolean) servletRequest.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED);
if (authInfoChecked == null && !errorOverridesMissingAuth(response.getStatus())) { if (authInfoChecked == null && statusIsSuccess(response.getStatus())) {
// Note: rather than throwing an exception here, it would be nice to blank out the original response // Note: rather than throwing an exception here, it would be nice to blank out the original response
// since the request didn't have any authorization checks performed. However, this breaks proxying // since the request didn't have any authorization checks performed. However, this breaks proxying
// (e.g. OverlordServletProxy), so this is not implemented for now. // (e.g. OverlordServletProxy), so this is not implemented for now.
@ -150,6 +150,8 @@ public class PreResponseAuthorizationCheckFilter implements Filter
log.makeAlert(errorMsg) log.makeAlert(errorMsg)
.addData("uri", servletRequest.getRequestURI()) .addData("uri", servletRequest.getRequestURI())
.addData("method", servletRequest.getMethod()) .addData("method", servletRequest.getMethod())
.addData("remoteAddr", servletRequest.getRemoteAddr())
.addData("remoteHost", servletRequest.getRemoteHost())
.emit(); .emit();
if (servletResponse.isCommitted()) { if (servletResponse.isCommitted()) {
@ -164,9 +166,9 @@ public class PreResponseAuthorizationCheckFilter implements Filter
} }
} }
private static boolean errorOverridesMissingAuth(int status) private static boolean statusIsSuccess(int status)
{ {
return status == Response.SC_INTERNAL_SERVER_ERROR; return 200 <= status && status < 300;
} }
public static void sendJsonError(HttpServletResponse resp, int error, String errorJson, OutputStream outputStream) public static void sendJsonError(HttpServletResponse resp, int error, String errorJson, OutputStream outputStream)

26
server/src/test/java/io/druid/server/http/security/PreResponseAuthorizationCheckFilterTest.java
View File

@ -115,6 +115,8 @@ public class PreResponseAuthorizationCheckFilterTest
EasyMock.expect(resp.getStatus()).andReturn(200).once(); EasyMock.expect(resp.getStatus()).andReturn(200).once();
EasyMock.expect(req.getRequestURI()).andReturn("uri").once(); EasyMock.expect(req.getRequestURI()).andReturn("uri").once();
EasyMock.expect(req.getMethod()).andReturn("GET").once(); EasyMock.expect(req.getMethod()).andReturn("GET").once();
EasyMock.expect(req.getRemoteAddr()).andReturn("1.2.3.4").once();
EasyMock.expect(req.getRemoteHost()).andReturn("ahostname").once();
EasyMock.expect(resp.isCommitted()).andReturn(true).once(); EasyMock.expect(resp.isCommitted()).andReturn(true).once();
resp.setStatus(403); resp.setStatus(403);
EasyMock.expectLastCall().once(); EasyMock.expectLastCall().once();
@ -131,4 +133,28 @@ public class PreResponseAuthorizationCheckFilterTest
filter.doFilter(req, resp, filterChain); filter.doFilter(req, resp, filterChain);
EasyMock.verify(req, resp, filterChain, outputStream); EasyMock.verify(req, resp, filterChain, outputStream);
} }
@Test
public void testMissingAuthorizationCheckWithError() throws Exception
{
EmittingLogger.registerEmitter(EasyMock.createNiceMock(ServiceEmitter.class));
AuthenticationResult authenticationResult = new AuthenticationResult("so-very-valid", "so-very-valid", null);
HttpServletRequest req = EasyMock.createStrictMock(HttpServletRequest.class);
HttpServletResponse resp = EasyMock.createStrictMock(HttpServletResponse.class);
FilterChain filterChain = EasyMock.createNiceMock(FilterChain.class);
ServletOutputStream outputStream = EasyMock.createNiceMock(ServletOutputStream.class);
EasyMock.expect(req.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)).andReturn(authenticationResult).once();
EasyMock.expect(req.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED)).andReturn(null).once();
EasyMock.expect(resp.getStatus()).andReturn(404).once();
EasyMock.replay(req, resp, filterChain, outputStream);
PreResponseAuthorizationCheckFilter filter = new PreResponseAuthorizationCheckFilter(
authenticators,
new DefaultObjectMapper()
);
filter.doFilter(req, resp, filterChain);
EasyMock.verify(req, resp, filterChain, outputStream);
}
} }