Skip to content

Commit

Permalink
Alternative solution to fix #59 and possibly #167
Browse files Browse the repository at this point in the history
  • Loading branch information
hazendaz committed Sep 14, 2024
1 parent 1cf49e2 commit 4880d29
Show file tree
Hide file tree
Showing 16 changed files with 45 additions and 90 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,7 @@ public IWindowsIdentity doFilter(final HttpServletRequest request, final HttpSer
Boolean.valueOf(securityContext.isContinue()));
if (securityContext.isContinue()) {
response.setHeader("Connection", "keep-alive");
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
final String body = "Unauthorized";
response.getWriter().write(body);
response.setContentLength(body.length());
response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
response.flushBuffer();
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,17 @@ protected boolean onLoginFailure(final AuthenticationToken token, final Authenti
// negotiate is processing
final String protocol = this.getAuthzHeaderProtocol(request);
NegotiateAuthenticationFilter.LOGGER.debug("Negotiation in progress for protocol: {}", protocol);
this.sendChallengeDuringNegotiate(protocol, response, ((NegotiateToken) token).getOut());
try {
this.sendChallengeDuringNegotiate(protocol, response, ((NegotiateToken) token).getOut());
} catch (IOException e1) {
NegotiateAuthenticationFilter.LOGGER.warn("login exception: {}", e1.getMessage());

// do not send token.out bytes, this was a login failure.
this.sendChallengeOnFailure(response);

this.setFailureAttribute(request, e);
return true;
}
return false;
}
NegotiateAuthenticationFilter.LOGGER.warn("login exception: {}", e.getMessage());
Expand Down Expand Up @@ -292,20 +302,25 @@ boolean isLoginAttempt(final String authzHeader) {
* outgoing ServletResponse
* @param out
* token.out or null
* @throws IOException
* Signals that an I/O exception has occurred.
*/
private void sendChallenge(final List<String> protocols, final ServletResponse response, final byte[] out) {
private void sendChallenge(final List<String> protocols, final ServletResponse response, final byte[] out)
throws IOException {
final HttpServletResponse httpResponse = WebUtils.toHttp(response);
this.sendAuthenticateHeader(protocols, out, httpResponse);
httpResponse.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED);
}

/**
* Send challenge initiate negotiate.
*
* @param response
* the response
* @throws IOException
* Signals that an I/O exception has occurred.
*/
void sendChallengeInitiateNegotiate(final ServletResponse response) {
void sendChallengeInitiateNegotiate(final ServletResponse response) throws IOException {
this.sendChallenge(NegotiateAuthenticationFilter.PROTOCOLS, response, null);
}

Expand All @@ -318,8 +333,11 @@ void sendChallengeInitiateNegotiate(final ServletResponse response) {
* the response
* @param out
* the out
* @throws IOException
* Signals that an I/O exception has occurred.
*/
void sendChallengeDuringNegotiate(final String protocol, final ServletResponse response, final byte[] out) {
void sendChallengeDuringNegotiate(final String protocol, final ServletResponse response, final byte[] out)
throws IOException {
final List<String> protocolsList = new ArrayList<>();
protocolsList.add(protocol);
this.sendChallenge(protocolsList, response, out);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,4 @@ public void setHeader(final String name, final String value) {
this.headers.put(name, value);
}

@Override
public void setStatus(final int status) {
this.sc = status;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
*/
package waffle.shiro.negotiate;

import java.io.IOException;
import java.util.Base64;
import java.util.HashMap;

Expand Down Expand Up @@ -76,6 +77,9 @@ void testIsLoginAttempt() {

/**
* Test send challenge during negotiate.
*
* @throws IOException
* Signals that an I/O exception has occurred.
*/
@Test
void testSendChallengeDuringNegotiate() {
Expand All @@ -92,14 +96,16 @@ void testSendChallengeDuringNegotiate() {

Assertions.assertEquals("keep-alive", this.response.headers.get("Connection"));

Assertions.assertEquals(HttpServletResponse.SC_UNAUTHORIZED, this.response.sc);
Assertions.assertEquals(0, this.response.errorCode);
Assertions.assertEquals(HttpServletResponse.SC_UNAUTHORIZED, this.response.errorCode);

Assertions.assertFalse(this.response.isFlushed);
}

/**
* Test send challenge initiate negotiate.
*
* @throws IOException
* Signals that an I/O exception has occurred.
*/
@Test
void testSendChallengeInitiateNegotiate() {
Expand All @@ -114,8 +120,7 @@ void testSendChallengeInitiateNegotiate() {

Assertions.assertEquals("keep-alive", this.response.headers.get("Connection"));

Assertions.assertEquals(HttpServletResponse.SC_UNAUTHORIZED, this.response.sc);
Assertions.assertEquals(0, this.response.errorCode);
Assertions.assertEquals(HttpServletResponse.SC_UNAUTHORIZED, this.response.errorCode);

Assertions.assertFalse(this.response.isFlushed);
}
Expand All @@ -133,7 +138,6 @@ void testSendChallengeOnFailure() {

Assertions.assertEquals("close", this.response.headers.get("Connection"));

Assertions.assertEquals(0, this.response.sc);
Assertions.assertEquals(HttpServletResponse.SC_UNAUTHORIZED, this.response.errorCode);

Assertions.assertTrue(this.response.isFlushed);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public void commence(final HttpServletRequest request, final HttpServletResponse
throw new ServletException("Missing NegotiateEntryPoint.Provider");
}

response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
response.setHeader("Connection", "keep-alive");
this.provider.sendUnauthorized(response);
response.flushBuffer();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,6 @@ public void setHeader(final String headerName, final String headerValue) {
this.headers.put(headerName, current);
}

@Override
public void setStatus(final int value) {
this.status = value;
}

/**
* Gets the status string.
*
Expand Down Expand Up @@ -213,9 +208,4 @@ public String getOutputText() {
return this.bytes.toString(StandardCharsets.UTF_8);
}

@Override
public void setContentLength(int len) {
setHeader("Content-Length", Integer.toString(len));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ void testChallengePOST() throws IOException, ServletException {
this.filter.doFilter(request, response, null);
Assertions.assertTrue(response.getHeader("WWW-Authenticate").startsWith(securityPackage + " "));
Assertions.assertEquals("keep-alive", response.getHeader("Connection"));
Assertions.assertEquals(3, response.getHeaderNamesSize());
Assertions.assertEquals(2, response.getHeaderNamesSize());
Assertions.assertEquals(401, response.getStatus());
} finally {
if (clientContext != null) {
Expand Down Expand Up @@ -206,26 +206,10 @@ void testNegotiate() throws IOException, ServletException {
break;
}

Assertions.assertEquals(401, response.getStatus());

// security package requested is one negotiate continues with
Assertions.assertTrue(response.getHeader("WWW-Authenticate").startsWith(securityPackage + " "));

// keep-alive, NTLM is a connection-oriented protocol
Assertions.assertEquals("keep-alive", response.getHeader("Connection"));

// Connection: keep-alive
// WWW-Authenticate: ...
// Content-Length: ...
Assertions.assertEquals(3, response.getHeaderNamesSize());

// response has a body and a content length (.NET clients require this)
int contentLength = Integer.parseInt(response.getHeader("Content-Length"));
Assertions.assertTrue(contentLength > 0);
String content = response.getOutputText();
Assertions.assertEquals(contentLength, content.length());

// continue token
Assertions.assertEquals(2, response.getHeaderNamesSize());
Assertions.assertEquals(401, response.getStatus());
final String continueToken = response.getHeader("WWW-Authenticate")
.substring(securityPackage.length() + 1);
final byte[] continueTokenBytes = Base64.getDecoder().decode(continueToken);
Expand Down Expand Up @@ -303,7 +287,7 @@ void testChallengeNTLMPOST() throws IOException, ServletException {
final String[] wwwAuthenticates = response.getHeaderValues("WWW-Authenticate");
Assertions.assertEquals(1, wwwAuthenticates.length);
Assertions.assertTrue(wwwAuthenticates[0].startsWith("NTLM "));
Assertions.assertEquals(3, response.getHeaderNamesSize());
Assertions.assertEquals(2, response.getHeaderNamesSize());
Assertions.assertEquals("keep-alive", response.getHeader("Connection"));
Assertions.assertEquals(401, response.getStatus());
}
Expand Down Expand Up @@ -332,7 +316,7 @@ void testChallengeNTLMPUT() throws IOException, ServletException {
final String[] wwwAuthenticates = response.getHeaderValues("WWW-Authenticate");
Assertions.assertEquals(1, wwwAuthenticates.length);
Assertions.assertTrue(wwwAuthenticates[0].startsWith("NTLM "));
Assertions.assertEquals(3, response.getHeaderNamesSize());
Assertions.assertEquals(2, response.getHeaderNamesSize());
Assertions.assertEquals("keep-alive", response.getHeader("Connection"));
Assertions.assertEquals(401, response.getStatus());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,7 @@ private boolean negotiate(final Request request, final HttpServletResponse respo
try {
if (securityContext.isContinue() || ntlmPost) {
response.setHeader("Connection", "keep-alive");
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
String body = "Unauthorized";
response.getWriter().write(body);
response.setContentLength(body.length());
response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
response.flushBuffer();
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,7 @@ public boolean authenticate(final Request request, final HttpServletResponse res
try {
if (securityContext.isContinue()) {
response.setHeader("Connection", "keep-alive");
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
final String body = "Unauthorized";
response.getWriter().write(body);
response.setContentLength(body.length());
response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
response.flushBuffer();
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,4 @@ public void setHeader(final String headerName, final String headerValue) {
this.headers.put(headerName, current);
}

@Override
public void setStatus(final int value) {
this.status = value;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,7 @@ private boolean negotiate(final Request request, final HttpServletResponse respo
try {
if (securityContext.isContinue() || ntlmPost) {
response.setHeader("Connection", "keep-alive");
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
String body = "Unauthorized";
response.getWriter().write(body);
response.setContentLength(body.length());
response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
response.flushBuffer();
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,7 @@ public boolean authenticate(final Request request, final HttpServletResponse res
try {
if (securityContext.isContinue()) {
response.setHeader("Connection", "keep-alive");
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
final String body = "Unauthorized";
response.getWriter().write(body);
response.setContentLength(body.length());
response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
response.flushBuffer();
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,4 @@ public void setHeader(final String headerName, final String headerValue) {
this.headers.put(headerName, current);
}

@Override
public void setStatus(final int value) {
this.status = value;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,7 @@ private boolean negotiate(final Request request, final HttpServletResponse respo
try {
if (securityContext.isContinue() || ntlmPost) {
response.setHeader("Connection", "keep-alive");
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
String body = "Unauthorized";
response.getWriter().write(body);
response.setContentLength(body.length());
response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
response.flushBuffer();
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,7 @@ public boolean authenticate(final Request request, final HttpServletResponse res
try {
if (securityContext.isContinue()) {
response.setHeader("Connection", "keep-alive");
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
final String body = "Unauthorized";
response.getWriter().write(body);
response.setContentLength(body.length());
response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
response.flushBuffer();
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,4 @@ public void setHeader(final String headerName, final String headerValue) {
this.headers.put(headerName, current);
}

@Override
public void setStatus(final int value) {
this.status = value;
}

}

0 comments on commit 4880d29

Please sign in to comment.