YARN-8247 Incorrect HTTP status code returned by ATSv2 for non-whitelisted users. Contributed by Rohith Sharma K S

(cherry picked from commit 3c95ca4f21)
This commit is contained in:
Vrushali C 2018-05-09 22:17:48 -07:00 committed by Rohith Sharma K S
parent 1bc12310d8
commit c2b05339cf
2 changed files with 44 additions and 28 deletions

View File

@ -27,15 +27,13 @@ import javax.servlet.ServletException;
import javax.servlet.ServletRequest; import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse; import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.core.Response; import javax.servlet.http.HttpServletResponse;
import javax.ws.rs.core.Response.Status;
import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.StringUtils;
import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.UserGroupInformation;
import org.apache.hadoop.security.authorize.AccessControlList; import org.apache.hadoop.security.authorize.AccessControlList;
import org.apache.hadoop.security.authorize.AuthorizationException; import org.apache.hadoop.security.authorize.AuthorizationException;
import org.apache.hadoop.yarn.conf.YarnConfiguration; import org.apache.hadoop.yarn.conf.YarnConfiguration;
import org.apache.hadoop.yarn.webapp.ForbiddenException;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import org.apache.hadoop.yarn.server.timelineservice.reader.TimelineReaderWebServicesUtils; import org.apache.hadoop.yarn.server.timelineservice.reader.TimelineReaderWebServicesUtils;
@ -64,9 +62,12 @@ public class TimelineReaderWhitelistAuthorizationFilter implements Filter {
@Override @Override
public void doFilter(ServletRequest request, ServletResponse response, public void doFilter(ServletRequest request, ServletResponse response,
FilterChain chain) throws IOException, ServletException { FilterChain chain) throws IOException, ServletException {
HttpServletRequest httpRequest = (HttpServletRequest) request;
HttpServletResponse httpResponse = (HttpServletResponse) response;
if (isWhitelistReadAuthEnabled) { if (isWhitelistReadAuthEnabled) {
UserGroupInformation callerUGI = TimelineReaderWebServicesUtils UserGroupInformation callerUGI = TimelineReaderWebServicesUtils
.getCallerUserGroupInformation((HttpServletRequest) request, true); .getCallerUserGroupInformation(httpRequest, true);
if (callerUGI == null) { if (callerUGI == null) {
String msg = "Unable to obtain user name, user not authenticated"; String msg = "Unable to obtain user name, user not authenticated";
throw new AuthorizationException(msg); throw new AuthorizationException(msg);
@ -76,9 +77,8 @@ public class TimelineReaderWhitelistAuthorizationFilter implements Filter {
String userName = callerUGI.getShortUserName(); String userName = callerUGI.getShortUserName();
String msg = "User " + userName String msg = "User " + userName
+ " is not allowed to read TimelineService V2 data."; + " is not allowed to read TimelineService V2 data.";
Response.status(Status.FORBIDDEN).entity(msg).build(); httpResponse.sendError(HttpServletResponse.SC_FORBIDDEN, msg);
throw new ForbiddenException("user " + userName return;
+ " is not allowed to read TimelineService V2 data");
} }
} }
if (chain != null) { if (chain != null) {

View File

@ -18,6 +18,7 @@
package org.apache.hadoop.yarn.server.timelineservice.reader; package org.apache.hadoop.yarn.server.timelineservice.reader;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
@ -32,13 +33,12 @@ import java.util.Map;
import javax.servlet.FilterConfig; import javax.servlet.FilterConfig;
import javax.servlet.ServletContext; import javax.servlet.ServletContext;
import javax.servlet.ServletException; import javax.servlet.ServletException;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.UserGroupInformation;
import org.apache.hadoop.yarn.conf.YarnConfiguration; import org.apache.hadoop.yarn.conf.YarnConfiguration;
import org.apache.hadoop.yarn.server.timelineservice.reader.security.TimelineReaderWhitelistAuthorizationFilter; import org.apache.hadoop.yarn.server.timelineservice.reader.security.TimelineReaderWhitelistAuthorizationFilter;
import org.apache.hadoop.yarn.webapp.ForbiddenException;
import org.junit.Test; import org.junit.Test;
import org.mockito.Mockito; import org.mockito.Mockito;
@ -100,11 +100,11 @@ public class TestTimelineReaderWhitelistAuthorizationFilter {
} }
}); });
final ServletResponse r = Mockito.mock(ServletResponse.class); HttpServletResponse r = Mockito.mock(HttpServletResponse.class);
f.doFilter(mockHsr, r, null); f.doFilter(mockHsr, r, null);
} }
@Test(expected = ForbiddenException.class) @Test
public void checkFilterNotAllowedUser() throws ServletException, IOException { public void checkFilterNotAllowedUser() throws ServletException, IOException {
Map<String, String> map = new HashMap<String, String>(); Map<String, String> map = new HashMap<String, String>();
map.put(YarnConfiguration.TIMELINE_SERVICE_READ_AUTH_ENABLED, "true"); map.put(YarnConfiguration.TIMELINE_SERVICE_READ_AUTH_ENABLED, "true");
@ -115,14 +115,20 @@ public class TestTimelineReaderWhitelistAuthorizationFilter {
FilterConfig fc = new DummyFilterConfig(map); FilterConfig fc = new DummyFilterConfig(map);
f.init(fc); f.init(fc);
final HttpServletRequest mockHsr = Mockito.mock(HttpServletRequest.class); final HttpServletRequest mockHsr = Mockito.mock(HttpServletRequest.class);
final String userName = "testuser1";
Mockito.when(mockHsr.getUserPrincipal()).thenReturn(new Principal() { Mockito.when(mockHsr.getUserPrincipal()).thenReturn(new Principal() {
@Override @Override
public String getName() { public String getName() {
return "testuser1"; return userName;
} }
}); });
final ServletResponse r = Mockito.mock(ServletResponse.class); HttpServletResponse r = Mockito.mock(HttpServletResponse.class);
f.doFilter(mockHsr, r, null); f.doFilter(mockHsr, r, null);
String msg = "User " + userName
+ " is not allowed to read TimelineService V2 data.";
Mockito.verify(r)
.sendError(eq(HttpServletResponse.SC_FORBIDDEN), eq(msg));
} }
@Test @Test
@ -143,7 +149,7 @@ public class TestTimelineReaderWhitelistAuthorizationFilter {
return "user1"; return "user1";
} }
}); });
final ServletResponse r = Mockito.mock(ServletResponse.class); final HttpServletResponse r = Mockito.mock(HttpServletResponse.class);
UserGroupInformation user1 = UserGroupInformation user1 =
UserGroupInformation.createUserForTesting("user1", GROUP_NAMES); UserGroupInformation.createUserForTesting("user1", GROUP_NAMES);
user1.doAs(new PrivilegedExceptionAction<Object>() { user1.doAs(new PrivilegedExceptionAction<Object>() {
@ -155,7 +161,7 @@ public class TestTimelineReaderWhitelistAuthorizationFilter {
}); });
} }
@Test(expected = ForbiddenException.class) @Test
public void checkFilterNotAlloweGroup() public void checkFilterNotAlloweGroup()
throws ServletException, IOException, InterruptedException { throws ServletException, IOException, InterruptedException {
Map<String, String> map = new HashMap<String, String>(); Map<String, String> map = new HashMap<String, String>();
@ -167,15 +173,16 @@ public class TestTimelineReaderWhitelistAuthorizationFilter {
FilterConfig fc = new DummyFilterConfig(map); FilterConfig fc = new DummyFilterConfig(map);
f.init(fc); f.init(fc);
final HttpServletRequest mockHsr = Mockito.mock(HttpServletRequest.class); final HttpServletRequest mockHsr = Mockito.mock(HttpServletRequest.class);
final String userName = "user200";
Mockito.when(mockHsr.getUserPrincipal()).thenReturn(new Principal() { Mockito.when(mockHsr.getUserPrincipal()).thenReturn(new Principal() {
@Override @Override
public String getName() { public String getName() {
return "user200"; return userName;
} }
}); });
final ServletResponse r = Mockito.mock(ServletResponse.class); final HttpServletResponse r = Mockito.mock(HttpServletResponse.class);
UserGroupInformation user1 = UserGroupInformation user1 =
UserGroupInformation.createUserForTesting("user200", GROUP_NAMES); UserGroupInformation.createUserForTesting(userName, GROUP_NAMES);
user1.doAs(new PrivilegedExceptionAction<Object>() { user1.doAs(new PrivilegedExceptionAction<Object>() {
@Override @Override
public Object run() throws Exception { public Object run() throws Exception {
@ -183,6 +190,10 @@ public class TestTimelineReaderWhitelistAuthorizationFilter {
return null; return null;
} }
}); });
String msg = "User " + userName
+ " is not allowed to read TimelineService V2 data.";
Mockito.verify(r)
.sendError(eq(HttpServletResponse.SC_FORBIDDEN), eq(msg));
} }
@Test @Test
@ -205,7 +216,7 @@ public class TestTimelineReaderWhitelistAuthorizationFilter {
return "user90"; return "user90";
} }
}); });
final ServletResponse r = Mockito.mock(ServletResponse.class); final HttpServletResponse r = Mockito.mock(HttpServletResponse.class);
UserGroupInformation user1 = UserGroupInformation user1 =
UserGroupInformation.createUserForTesting("user90", GROUP_NAMES); UserGroupInformation.createUserForTesting("user90", GROUP_NAMES);
user1.doAs(new PrivilegedExceptionAction<Object>() { user1.doAs(new PrivilegedExceptionAction<Object>() {
@ -235,7 +246,7 @@ public class TestTimelineReaderWhitelistAuthorizationFilter {
return "user90"; return "user90";
} }
}); });
final ServletResponse r = Mockito.mock(ServletResponse.class); final HttpServletResponse r = Mockito.mock(HttpServletResponse.class);
UserGroupInformation user1 = UserGroupInformation user1 =
UserGroupInformation.createUserForTesting("user90", GROUP_NAMES); UserGroupInformation.createUserForTesting("user90", GROUP_NAMES);
user1.doAs(new PrivilegedExceptionAction<Object>() { user1.doAs(new PrivilegedExceptionAction<Object>() {
@ -247,7 +258,7 @@ public class TestTimelineReaderWhitelistAuthorizationFilter {
}); });
} }
@Test(expected = ForbiddenException.class) @Test
public void checkFilterAllowNoOneWhenAdminAclsEmptyAndUserAclsEmpty() public void checkFilterAllowNoOneWhenAdminAclsEmptyAndUserAclsEmpty()
throws ServletException, IOException, InterruptedException { throws ServletException, IOException, InterruptedException {
// check that users in admin acl list are allowed to read // check that users in admin acl list are allowed to read
@ -258,15 +269,16 @@ public class TestTimelineReaderWhitelistAuthorizationFilter {
FilterConfig fc = new DummyFilterConfig(map); FilterConfig fc = new DummyFilterConfig(map);
f.init(fc); f.init(fc);
final HttpServletRequest mockHsr = Mockito.mock(HttpServletRequest.class); final HttpServletRequest mockHsr = Mockito.mock(HttpServletRequest.class);
final String userName = "user88";
Mockito.when(mockHsr.getUserPrincipal()).thenReturn(new Principal() { Mockito.when(mockHsr.getUserPrincipal()).thenReturn(new Principal() {
@Override @Override
public String getName() { public String getName() {
return "user88"; return userName;
} }
}); });
final ServletResponse r = Mockito.mock(ServletResponse.class); final HttpServletResponse r = Mockito.mock(HttpServletResponse.class);
UserGroupInformation user1 = UserGroupInformation user1 =
UserGroupInformation.createUserForTesting("user88", GROUP_NAMES); UserGroupInformation.createUserForTesting(userName, GROUP_NAMES);
user1.doAs(new PrivilegedExceptionAction<Object>() { user1.doAs(new PrivilegedExceptionAction<Object>() {
@Override @Override
public Object run() throws Exception { public Object run() throws Exception {
@ -274,6 +286,10 @@ public class TestTimelineReaderWhitelistAuthorizationFilter {
return null; return null;
} }
}); });
String msg = "User " + userName
+ " is not allowed to read TimelineService V2 data.";
Mockito.verify(r)
.sendError(eq(HttpServletResponse.SC_FORBIDDEN), eq(msg));
} }
@Test @Test
@ -293,7 +309,7 @@ public class TestTimelineReaderWhitelistAuthorizationFilter {
return "user437"; return "user437";
} }
}); });
final ServletResponse r = Mockito.mock(ServletResponse.class); final HttpServletResponse r = Mockito.mock(HttpServletResponse.class);
UserGroupInformation user1 = UserGroupInformation user1 =
UserGroupInformation.createUserForTesting("user437", GROUP_NAMES); UserGroupInformation.createUserForTesting("user437", GROUP_NAMES);
user1.doAs(new PrivilegedExceptionAction<Object>() { user1.doAs(new PrivilegedExceptionAction<Object>() {
@ -327,7 +343,7 @@ public class TestTimelineReaderWhitelistAuthorizationFilter {
} }
}); });
final ServletResponse r = Mockito.mock(ServletResponse.class); final HttpServletResponse r = Mockito.mock(HttpServletResponse.class);
UserGroupInformation user1 = UserGroupInformation user1 =
// both username and group name are not part of admin and // both username and group name are not part of admin and
// read allowed users // read allowed users
@ -348,7 +364,7 @@ public class TestTimelineReaderWhitelistAuthorizationFilter {
return "user27"; return "user27";
} }
}); });
final ServletResponse r2 = Mockito.mock(ServletResponse.class); final HttpServletResponse r2 = Mockito.mock(HttpServletResponse.class);
UserGroupInformation user2 = UserGroupInformation user2 =
UserGroupInformation.createUserForTesting("user27", GROUP_NAMES); UserGroupInformation.createUserForTesting("user27", GROUP_NAMES);
user2.doAs(new PrivilegedExceptionAction<Object>() { user2.doAs(new PrivilegedExceptionAction<Object>() {
@ -366,7 +382,7 @@ public class TestTimelineReaderWhitelistAuthorizationFilter {
return "user2"; return "user2";
} }
}); });
final ServletResponse r3 = Mockito.mock(ServletResponse.class); final HttpServletResponse r3 = Mockito.mock(HttpServletResponse.class);
UserGroupInformation user3 = UserGroupInformation user3 =
UserGroupInformation.createUserForTesting("user2", GROUP_NAMES); UserGroupInformation.createUserForTesting("user2", GROUP_NAMES);
user3.doAs(new PrivilegedExceptionAction<Object>() { user3.doAs(new PrivilegedExceptionAction<Object>() {