From e29ede3f729784f0eb770f0a1570bea199ff6902 Mon Sep 17 00:00:00 2001 From: Alejandro Abdelnur Date: Wed, 25 Apr 2012 03:20:53 +0000 Subject: [PATCH] HADOOP-8314. HttpServer#hasAdminAccess should return false if authorization is enabled but user is not authenticated. (tucu) git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1330086 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-common/CHANGES.txt | 3 ++ .../org/apache/hadoop/http/HttpServer.java | 9 ++-- .../apache/hadoop/http/TestHttpServer.java | 45 +++++++++++++++++++ 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 043a9e8440..959fe510e8 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -375,6 +375,9 @@ Release 2.0.0 - UNRELEASED HADOOP-8309. Pseudo & Kerberos AuthenticationHandler should use getType() to create token (tucu) + HADOOP-8314. HttpServer#hasAdminAccess should return false if + authorization is enabled but user is not authenticated. (tucu) + BREAKDOWN OF HADOOP-7454 SUBTASKS HADOOP-7455. HA: Introduce HA Service Protocol Interface. (suresh) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer.java index 979ec7656e..ad83016187 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer.java @@ -821,7 +821,10 @@ public static boolean hasAdministratorAccess( String remoteUser = request.getRemoteUser(); if (remoteUser == null) { - return true; + response.sendError(HttpServletResponse.SC_UNAUTHORIZED, + "Unauthenticated users are not " + + "authorized to access this page."); + return false; } AccessControlList adminsAcl = (AccessControlList) servletContext .getAttribute(ADMINS_ACL); @@ -830,9 +833,7 @@ public static boolean hasAdministratorAccess( if (adminsAcl != null) { if (!adminsAcl.isUserAllowed(remoteUserUGI)) { response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "User " - + remoteUser + " is unauthorized to access this page. " - + "AccessControlList for accessing this page : " - + adminsAcl.toString()); + + remoteUser + " is unauthorized to access this page."); return false; } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServer.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServer.java index bf2f3fecc6..f56231b64f 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServer.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServer.java @@ -35,6 +35,7 @@ import javax.servlet.Filter; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; +import javax.servlet.ServletContext; import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; @@ -53,10 +54,12 @@ import org.apache.hadoop.http.resource.JerseyResource; import org.apache.hadoop.security.Groups; import org.apache.hadoop.security.ShellBasedUnixGroupsMapping; +import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.authorize.AccessControlList; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; +import org.mockito.Mock; import org.mockito.Mockito; import org.mortbay.util.ajax.JSON; @@ -422,4 +425,46 @@ private static Map parse(String jsonString) { assertEquals("bar", m.get(JerseyResource.OP)); LOG.info("END testJersey()"); } + + @Test + public void testHasAdministratorAccess() throws Exception { + Configuration conf = new Configuration(); + conf.setBoolean(CommonConfigurationKeys.HADOOP_SECURITY_AUTHORIZATION, false); + ServletContext context = Mockito.mock(ServletContext.class); + Mockito.when(context.getAttribute(HttpServer.CONF_CONTEXT_ATTRIBUTE)).thenReturn(conf); + Mockito.when(context.getAttribute(HttpServer.ADMINS_ACL)).thenReturn(null); + HttpServletRequest request = Mockito.mock(HttpServletRequest.class); + Mockito.when(request.getRemoteUser()).thenReturn(null); + HttpServletResponse response = Mockito.mock(HttpServletResponse.class); + + //authorization OFF + Assert.assertTrue(HttpServer.hasAdministratorAccess(context, request, response)); + + //authorization ON & user NULL + response = Mockito.mock(HttpServletResponse.class); + conf.setBoolean(CommonConfigurationKeys.HADOOP_SECURITY_AUTHORIZATION, true); + Assert.assertFalse(HttpServer.hasAdministratorAccess(context, request, response)); + Mockito.verify(response).sendError(Mockito.eq(HttpServletResponse.SC_UNAUTHORIZED), Mockito.anyString()); + + //authorization ON & user NOT NULL & ACLs NULL + response = Mockito.mock(HttpServletResponse.class); + Mockito.when(request.getRemoteUser()).thenReturn("foo"); + Assert.assertTrue(HttpServer.hasAdministratorAccess(context, request, response)); + + //authorization ON & user NOT NULL & ACLs NOT NULL & user not in ACLs + response = Mockito.mock(HttpServletResponse.class); + AccessControlList acls = Mockito.mock(AccessControlList.class); + Mockito.when(acls.isUserAllowed(Mockito.any())).thenReturn(false); + Mockito.when(context.getAttribute(HttpServer.ADMINS_ACL)).thenReturn(acls); + Assert.assertFalse(HttpServer.hasAdministratorAccess(context, request, response)); + Mockito.verify(response).sendError(Mockito.eq(HttpServletResponse.SC_UNAUTHORIZED), Mockito.anyString()); + + //authorization ON & user NOT NULL & ACLs NOT NULL & user in in ACLs + response = Mockito.mock(HttpServletResponse.class); + Mockito.when(acls.isUserAllowed(Mockito.any())).thenReturn(true); + Mockito.when(context.getAttribute(HttpServer.ADMINS_ACL)).thenReturn(acls); + Assert.assertTrue(HttpServer.hasAdministratorAccess(context, request, response)); + + } + }