From 004d0854b7964d4f748f6e91b2d54a84928843f7 Mon Sep 17 00:00:00 2001 From: Arun Murthy Date: Mon, 31 Mar 2014 07:25:53 +0000 Subject: [PATCH] HDFS-4564. Ensure webhdfs returns correct HTTP response codes for denied operations. Contributed by Daryn Sharp. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1583241 13f79535-47bb-0310-9956-ffa450edef68 --- .../fs/http/server/TestHttpFSServer.java | 2 +- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 ++ .../hadoop/hdfs/web/WebHdfsFileSystem.java | 50 ++++++++----------- .../hdfs/web/resources/ExceptionHandler.java | 4 +- .../web/TestWebHdfsFileSystemContract.java | 2 +- 5 files changed, 27 insertions(+), 34 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java index 6057a48021..48cca42e57 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java +++ b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java @@ -349,7 +349,7 @@ public void testDelegationTokenOperations() throws Exception { url = new URL(TestJettyHelper.getJettyURL(), "/webhdfs/v1/?op=GETHOMEDIRECTORY&delegation=" + tokenStr); conn = (HttpURLConnection) url.openConnection(); - Assert.assertEquals(HttpURLConnection.HTTP_UNAUTHORIZED, + Assert.assertEquals(HttpURLConnection.HTTP_FORBIDDEN, conn.getResponseCode()); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index e3e4cb664f..068ce5df1b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -1050,6 +1050,9 @@ BREAKDOWN OF HDFS-5535 ROLLING UPGRADE SUBTASKS AND RELATED JIRAS HDFS-6038. Allow JournalNode to handle editlog produced by new release with future layoutversion. (jing9) + HDFS-4564. Ensure webhdfs returns correct HTTP response codes for denied + operations. (daryn via acmurthy) + Release 2.3.1 - UNRELEASED INCOMPATIBLE CHANGES diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java index 90ec089f3a..f836d4d052 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java @@ -304,6 +304,11 @@ private Path makeAbsolute(Path f) { private static Map validateResponse(final HttpOpParam.Op op, final HttpURLConnection conn, boolean unwrapException) throws IOException { final int code = conn.getResponseCode(); + // server is demanding an authentication we don't support + if (code == HttpURLConnection.HTTP_UNAUTHORIZED) { + throw new IOException( + new AuthenticationException(conn.getResponseMessage())); + } if (code != op.getExpectedHttpResponseCode()) { final Map m; try { @@ -450,52 +455,33 @@ protected AbstractRunner(final HttpOpParam.Op op, boolean redirected) { this.redirected = redirected; } - private HttpURLConnection getHttpUrlConnection(final URL url) - throws IOException, AuthenticationException { + AbstractRunner run() throws IOException { UserGroupInformation connectUgi = ugi.getRealUser(); if (connectUgi == null) { connectUgi = ugi; } + if (op.getRequireAuth()) { + connectUgi.checkTGTAndReloginFromKeytab(); + } try { + // the entire lifecycle of the connection must be run inside the + // doAs to ensure authentication is performed correctly return connectUgi.doAs( - new PrivilegedExceptionAction() { + new PrivilegedExceptionAction() { @Override - public HttpURLConnection run() throws IOException { - return openHttpUrlConnection(url); + public AbstractRunner run() throws IOException { + return runWithRetry(); } }); - } catch (IOException ioe) { - Throwable cause = ioe.getCause(); - if (cause != null && cause instanceof AuthenticationException) { - throw (AuthenticationException)cause; - } - throw ioe; } catch (InterruptedException e) { throw new IOException(e); } } - private HttpURLConnection openHttpUrlConnection(final URL url) - throws IOException { - final HttpURLConnection conn; - try { - conn = (HttpURLConnection) connectionFactory.openConnection(url, - op.getRequireAuth()); - } catch (AuthenticationException e) { - throw new IOException(e); - } - return conn; - } - private void init() throws IOException { checkRetry = !redirected; URL url = getUrl(); - try { - conn = getHttpUrlConnection(url); - } catch(AuthenticationException ae) { - checkRetry = false; - throw new IOException("Authentication failed, url=" + url, ae); - } + conn = (HttpURLConnection) connectionFactory.openConnection(url); } private void connect() throws IOException { @@ -516,7 +502,7 @@ private void disconnect() { } } - AbstractRunner run() throws IOException { + private AbstractRunner runWithRetry() throws IOException { /** * Do the real work. * @@ -543,6 +529,10 @@ AbstractRunner run() throws IOException { } return this; } catch(IOException ioe) { + Throwable cause = ioe.getCause(); + if (cause != null && cause instanceof AuthenticationException) { + throw ioe; // no retries for auth failures + } shouldRetry(ioe, retry); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/ExceptionHandler.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/ExceptionHandler.java index adb944a0dc..8456cc8700 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/ExceptionHandler.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/ExceptionHandler.java @@ -77,9 +77,9 @@ public Response toResponse(Exception e) { //Map response status final Response.Status s; if (e instanceof SecurityException) { - s = Response.Status.UNAUTHORIZED; + s = Response.Status.FORBIDDEN; } else if (e instanceof AuthorizationException) { - s = Response.Status.UNAUTHORIZED; + s = Response.Status.FORBIDDEN; } else if (e instanceof FileNotFoundException) { s = Response.Status.NOT_FOUND; } else if (e instanceof IOException) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHdfsFileSystemContract.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHdfsFileSystemContract.java index 9a0245cff8..867886148f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHdfsFileSystemContract.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHdfsFileSystemContract.java @@ -410,7 +410,7 @@ public void testResponseCode() throws IOException { new DoAsParam(ugi.getShortUserName() + "proxy")); final HttpURLConnection conn = (HttpURLConnection) url.openConnection(); conn.connect(); - assertEquals(HttpServletResponse.SC_UNAUTHORIZED, conn.getResponseCode()); + assertEquals(HttpServletResponse.SC_FORBIDDEN, conn.getResponseCode()); conn.disconnect(); }