Skip to content

Commit 0ebdce3

Browse files
Jing9Jing9
committed
HADOOP-10301. AuthenticationFilter should return Forbidden for failed authentication. Contributed by Daryn Sharp.
git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1582883 13f79535-47bb-0310-9956-ffa450edef68
1 parent 24d75b2 commit 0ebdce3

File tree

4 files changed

+112
-21
lines changed

4 files changed

+112
-21
lines changed

‎hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/AuthenticationFilter.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,8 @@ protected AuthenticationToken getToken(HttpServletRequest request) throws IOExce
332332
public void doFilter(ServletRequest request, ServletResponse response, FilterChain filterChain)
333333
throws IOException, ServletException {
334334
boolean unauthorizedResponse = true;
335-
String unauthorizedMsg = "";
335+
int errCode = HttpServletResponse.SC_UNAUTHORIZED;
336+
AuthenticationException authenticationEx = null;
336337
HttpServletRequest httpRequest = (HttpServletRequest) request;
337338
HttpServletResponse httpResponse = (HttpServletResponse) response;
338339
boolean isHttps = "https".equals(httpRequest.getScheme());
@@ -344,6 +345,8 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
344345
}
345346
catch (AuthenticationException ex) {
346347
LOG.warn("AuthenticationToken ignored: " + ex.getMessage());
348+
// will be sent back in a 401 unless filter authenticates
349+
authenticationEx = ex;
347350
token = null;
348351
}
349352
if (authHandler.managementOperation(token, httpRequest, httpResponse)) {
@@ -392,15 +395,20 @@ public Principal getUserPrincipal() {
392395
unauthorizedResponse = false;
393396
}
394397
} catch (AuthenticationException ex) {
395-
unauthorizedMsg = ex.toString();
398+
// exception from the filter itself is fatal
399+
errCode = HttpServletResponse.SC_FORBIDDEN;
400+
authenticationEx = ex;
396401
LOG.warn("Authentication exception: " + ex.getMessage(), ex);
397402
}
398403
if (unauthorizedResponse) {
399404
if (!httpResponse.isCommitted()) {
400405
createAuthCookie(httpResponse, "", getCookieDomain(),
401406
getCookiePath(), 0, isHttps);
402-
httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED,
403-
unauthorizedMsg);
407+
if (authenticationEx == null) {
408+
httpResponse.sendError(errCode, "Authentication required");
409+
} else {
410+
httpResponse.sendError(errCode, authenticationEx.getMessage());
411+
}
404412
}
405413
}
406414
}

‎hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/client/TestPseudoAuthenticator.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ public void testAnonymousDisallowed() throws Exception {
6363
URL url = new URL(auth.getBaseURL());
6464
HttpURLConnection conn = (HttpURLConnection) url.openConnection();
6565
conn.connect();
66-
Assert.assertEquals(HttpURLConnection.HTTP_UNAUTHORIZED, conn.getResponseCode());
66+
Assert.assertEquals(HttpURLConnection.HTTP_FORBIDDEN, conn.getResponseCode());
67+
Assert.assertEquals("Anonymous requests are disallowed", conn.getResponseMessage());
6768
} finally {
6869
auth.stop();
6970
}

‎hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/server/TestAuthenticationFilter.java‎

Lines changed: 95 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@
1414
package org.apache.hadoop.security.authentication.server;
1515

1616
import java.io.IOException;
17+
import java.net.HttpCookie;
1718
import java.util.Arrays;
1819
import java.util.HashMap;
20+
import java.util.List;
1921
import java.util.Properties;
2022
import java.util.Vector;
2123

@@ -130,7 +132,11 @@ public AuthenticationToken authenticate(HttpServletRequest request, HttpServletR
130132
token = new AuthenticationToken("u", "p", "t");
131133
token.setExpires((expired) ? 0 : System.currentTimeMillis() + TOKEN_VALIDITY_SEC);
132134
} else {
133-
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
135+
if (request.getHeader("WWW-Authenticate") == null) {
136+
response.setHeader("WWW-Authenticate", "dummyauth");
137+
} else {
138+
throw new AuthenticationException("AUTH FAILED");
139+
}
134140
}
135141
return token;
136142
}
@@ -303,7 +309,8 @@ public void testGetTokenExpired() throws Exception {
303309
"management.operation.return")).elements());
304310
filter.init(config);
305311

306-
AuthenticationToken token = new AuthenticationToken("u", "p", "invalidtype");
312+
AuthenticationToken token =
313+
new AuthenticationToken("u", "p", DummyAuthenticationHandler.TYPE);
307314
token.setExpires(System.currentTimeMillis() - TOKEN_VALIDITY_SEC);
308315
Signer signer = new Signer("secret".getBytes());
309316
String tokenSigned = signer.sign(token.toString());
@@ -312,13 +319,14 @@ public void testGetTokenExpired() throws Exception {
312319
HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
313320
Mockito.when(request.getCookies()).thenReturn(new Cookie[]{cookie});
314321

322+
boolean failed = false;
315323
try {
316324
filter.getToken(request);
317-
Assert.fail();
318325
} catch (AuthenticationException ex) {
319-
// Expected
320-
} catch (Exception ex) {
321-
Assert.fail();
326+
Assert.assertEquals("AuthenticationToken expired", ex.getMessage());
327+
failed = true;
328+
} finally {
329+
Assert.assertTrue("token not expired", failed);
322330
}
323331
} finally {
324332
filter.destroy();
@@ -351,13 +359,14 @@ public void testGetTokenInvalidType() throws Exception {
351359
HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
352360
Mockito.when(request.getCookies()).thenReturn(new Cookie[]{cookie});
353361

362+
boolean failed = false;
354363
try {
355364
filter.getToken(request);
356-
Assert.fail();
357365
} catch (AuthenticationException ex) {
358-
// Expected
359-
} catch (Exception ex) {
360-
Assert.fail();
366+
Assert.assertEquals("Invalid AuthenticationToken type", ex.getMessage());
367+
failed = true;
368+
} finally {
369+
Assert.assertTrue("token not invalid type", failed);
361370
}
362371
} finally {
363372
filter.destroy();
@@ -398,7 +407,9 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
398407

399408
filter.doFilter(request, response, chain);
400409

401-
Mockito.verify(response).setStatus(HttpServletResponse.SC_UNAUTHORIZED);
410+
Mockito.verify(response).sendError(
411+
HttpServletResponse.SC_UNAUTHORIZED, "Authentication required");
412+
Mockito.verify(response).setHeader("WWW-Authenticate", "dummyauth");
402413
} finally {
403414
filter.destroy();
404415
}
@@ -468,10 +479,10 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
468479

469480
if (expired) {
470481
Mockito.verify(response, Mockito.never()).
471-
addCookie(Mockito.any(Cookie.class));
482+
addHeader(Mockito.eq("Set-Cookie"), Mockito.anyString());
472483
} else {
473484
String v = cookieMap.get(AuthenticatedURL.AUTH_COOKIE);
474-
Assert.assertNotNull(v);
485+
Assert.assertNotNull("cookie missing", v);
475486
Assert.assertTrue(v.contains("u=") && v.contains("p=") && v.contains
476487
("t=") && v.contains("e=") && v.contains("s="));
477488
Mockito.verify(chain).doFilter(Mockito.any(ServletRequest.class),
@@ -585,18 +596,81 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
585596
}
586597
}
587598

599+
@Test
600+
public void testDoFilterAuthenticationFailure() throws Exception {
601+
AuthenticationFilter filter = new AuthenticationFilter();
602+
try {
603+
FilterConfig config = Mockito.mock(FilterConfig.class);
604+
Mockito.when(config.getInitParameter("management.operation.return")).
605+
thenReturn("true");
606+
Mockito.when(config.getInitParameter(AuthenticationFilter.AUTH_TYPE)).thenReturn(
607+
DummyAuthenticationHandler.class.getName());
608+
Mockito.when(config.getInitParameterNames()).thenReturn(
609+
new Vector<String>(
610+
Arrays.asList(AuthenticationFilter.AUTH_TYPE,
611+
"management.operation.return")).elements());
612+
filter.init(config);
613+
614+
HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
615+
Mockito.when(request.getRequestURL()).thenReturn(new StringBuffer("http://foo:8080/bar"));
616+
Mockito.when(request.getCookies()).thenReturn(new Cookie[]{});
617+
Mockito.when(request.getHeader("WWW-Authenticate")).thenReturn("dummyauth");
618+
HttpServletResponse response = Mockito.mock(HttpServletResponse.class);
619+
620+
FilterChain chain = Mockito.mock(FilterChain.class);
621+
622+
final HashMap<String, String> cookieMap = new HashMap<String, String>();
623+
Mockito.doAnswer(
624+
new Answer<Object>() {
625+
@Override
626+
public Object answer(InvocationOnMock invocation) throws Throwable {
627+
Object[] args = invocation.getArguments();
628+
parseCookieMap((String) args[1], cookieMap);
629+
return null;
630+
}
631+
}
632+
).when(response).addHeader(Mockito.eq("Set-Cookie"), Mockito.anyString());
633+
634+
Mockito.doAnswer(
635+
new Answer<Object>() {
636+
@Override
637+
public Object answer(InvocationOnMock invocation) throws Throwable {
638+
Assert.fail("shouldn't get here");
639+
return null;
640+
}
641+
}
642+
).when(chain).doFilter(Mockito.<ServletRequest>anyObject(), Mockito.<ServletResponse>anyObject());
643+
644+
filter.doFilter(request, response, chain);
645+
646+
Mockito.verify(response).sendError(
647+
HttpServletResponse.SC_FORBIDDEN, "AUTH FAILED");
648+
Mockito.verify(response, Mockito.never()).setHeader(Mockito.eq("WWW-Authenticate"), Mockito.anyString());
649+
650+
String value = cookieMap.get(AuthenticatedURL.AUTH_COOKIE);
651+
Assert.assertNotNull("cookie missing", value);
652+
Assert.assertEquals("", value);
653+
} finally {
654+
filter.destroy();
655+
}
656+
}
657+
588658
@Test
589659
public void testDoFilterAuthenticatedExpired() throws Exception {
660+
String secret = "secret";
590661
AuthenticationFilter filter = new AuthenticationFilter();
591662
try {
592663
FilterConfig config = Mockito.mock(FilterConfig.class);
593664
Mockito.when(config.getInitParameter("management.operation.return")).
594665
thenReturn("true");
595666
Mockito.when(config.getInitParameter(AuthenticationFilter.AUTH_TYPE)).thenReturn(
596667
DummyAuthenticationHandler.class.getName());
668+
Mockito.when(config.getInitParameter(AuthenticationFilter.SIGNATURE_SECRET)).thenReturn(
669+
secret);
597670
Mockito.when(config.getInitParameterNames()).thenReturn(
598671
new Vector<String>(
599672
Arrays.asList(AuthenticationFilter.AUTH_TYPE,
673+
AuthenticationFilter.SIGNATURE_SECRET,
600674
"management.operation.return")).elements());
601675
filter.init(config);
602676

@@ -605,7 +679,7 @@ public void testDoFilterAuthenticatedExpired() throws Exception {
605679

606680
AuthenticationToken token = new AuthenticationToken("u", "p", DummyAuthenticationHandler.TYPE);
607681
token.setExpires(System.currentTimeMillis() - TOKEN_VALIDITY_SEC);
608-
Signer signer = new Signer("secret".getBytes());
682+
Signer signer = new Signer(secret.getBytes());
609683
String tokenSigned = signer.sign(token.toString());
610684

611685
Cookie cookie = new Cookie(AuthenticatedURL.AUTH_COOKIE, tokenSigned);
@@ -643,22 +717,27 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
643717
Mockito.verify(chain, Mockito.never()).doFilter(Mockito.any
644718
(ServletRequest.class), Mockito.any(ServletResponse.class));
645719

646-
Assert.assertTrue(cookieMap.containsKey(AuthenticatedURL.AUTH_COOKIE));
720+
Assert.assertTrue("cookie is missing",
721+
cookieMap.containsKey(AuthenticatedURL.AUTH_COOKIE));
647722
Assert.assertEquals("", cookieMap.get(AuthenticatedURL.AUTH_COOKIE));
648723
}
649724

650725
@Test
651726
public void testDoFilterAuthenticatedInvalidType() throws Exception {
727+
String secret = "secret";
652728
AuthenticationFilter filter = new AuthenticationFilter();
653729
try {
654730
FilterConfig config = Mockito.mock(FilterConfig.class);
655731
Mockito.when(config.getInitParameter("management.operation.return")).
656732
thenReturn("true");
657733
Mockito.when(config.getInitParameter(AuthenticationFilter.AUTH_TYPE)).thenReturn(
658734
DummyAuthenticationHandler.class.getName());
735+
Mockito.when(config.getInitParameter(AuthenticationFilter.SIGNATURE_SECRET)).thenReturn(
736+
secret);
659737
Mockito.when(config.getInitParameterNames()).thenReturn(
660738
new Vector<String>(
661739
Arrays.asList(AuthenticationFilter.AUTH_TYPE,
740+
AuthenticationFilter.SIGNATURE_SECRET,
662741
"management.operation.return")).elements());
663742
filter.init(config);
664743

@@ -667,7 +746,7 @@ public void testDoFilterAuthenticatedInvalidType() throws Exception {
667746

668747
AuthenticationToken token = new AuthenticationToken("u", "p", "invalidtype");
669748
token.setExpires(System.currentTimeMillis() + TOKEN_VALIDITY_SEC);
670-
Signer signer = new Signer("secret".getBytes());
749+
Signer signer = new Signer(secret.getBytes());
671750
String tokenSigned = signer.sign(token.toString());
672751

673752
Cookie cookie = new Cookie(AuthenticatedURL.AUTH_COOKIE, tokenSigned);

‎hadoop-common-project/hadoop-common/CHANGES.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,9 @@ Release 2.4.0 - UNRELEASED
472472
HADOOP-10450. Build zlib native code bindings in hadoop.dll for Windows.
473473
(cnauroth)
474474

475+
HADOOP-10301. AuthenticationFilter should return Forbidden for failed
476+
authentication. (Daryn Sharp via jing9)
477+
475478
BREAKDOWN OF HADOOP-10184 SUBTASKS AND RELATED JIRAS
476479

477480
HADOOP-10185. FileSystem API for ACLs. (cnauroth)

0 commit comments

Comments
 (0)