Skip to content

Commit

Permalink
Merge pull request #6275 from jakewins/3.0-cors
Browse files Browse the repository at this point in the history 
Send real HTTP basic challenge unless browser sends special header
  • Loading branch information
@oskarhane
oskarhane committed Jan 22, 2016
2 parents 3744fdc + dc64e7f commit c8cccc7
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 128 deletions.
180 changes: 65 additions & 115 deletions community/server/src/main/java/org/neo4j/server/rest/dbms/AuthorizationFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.UriBuilder;

import org.neo4j.function.ThrowingConsumer;
import org.neo4j.kernel.api.exceptions.Status;
import org.neo4j.logging.Log;
import org.neo4j.logging.LogProvider;
Expand All @@ -42,7 +43,7 @@
import org.neo4j.server.web.XForwardUtil;

import static java.lang.String.format;
import static java.util.Arrays.asList;
import static java.util.Collections.singletonList;
import static javax.servlet.http.HttpServletRequest.BASIC_AUTH;
import static org.neo4j.helpers.collection.MapUtil.map;
import static org.neo4j.server.web.XForwardUtil.X_FORWARD_HOST_HEADER_KEY;
Expand Down Expand Up @@ -88,14 +89,14 @@ public void doFilter( ServletRequest servletRequest, ServletResponse servletResp
final String header = request.getHeader( HttpHeaders.AUTHORIZATION );
if ( header == null )
{
noHeader().writeResponse( response );
requestAuthentication( request, noHeader ).accept( response );
return;
}

final String[] usernameAndPassword = extractCredential( header );
if ( usernameAndPassword == null )
{
badHeader().writeResponse( response );
badHeader.accept( response );
return;
}

Expand All @@ -107,140 +108,89 @@ public void doFilter( ServletRequest servletRequest, ServletResponse servletResp
case PASSWORD_CHANGE_REQUIRED:
if ( !PASSWORD_CHANGE_WHITELIST.matcher( path ).matches() )
{
passwordChangeRequired( username, baseURL( request ) ).writeResponse( response );
passwordChangeRequired( username, baseURL( request ) ).accept( response );
return;
}
// fall through
case SUCCESS:
filterChain.doFilter( new AuthorizedRequestWrapper( BASIC_AUTH, username, request ), servletResponse );
return;
case TOO_MANY_ATTEMPTS:
tooManyAttemptes().writeResponse( response );
tooManyAttempts.accept( response );
return;
default:
log.warn( "Failed authentication attempt for '%s' from %s", username, request.getRemoteAddr() );
invalidCredential().writeResponse( response );
requestAuthentication( request, invalidCredential ).accept( response );
return;
}
}

// This is a pretty annoying duplication of work, where we're manually re-implementing the JSON serialization
// layer. Because we don't have access to jersey at this level, we can't use our regular serialization. This,
// obviously, implies a larger architectural issue, which is left as a future exercise.
private static abstract class ErrorResponse
private static ThrowingConsumer<HttpServletResponse, IOException> error( int code, Object body )
{
private final int statusCode;

private ErrorResponse( int statusCode )
{
this.statusCode = statusCode;
}

void addHeaders( HttpServletResponse response )
{
}

abstract Object body();

void writeResponse( HttpServletResponse response ) throws IOException
return (response) ->
{
response.setStatus( statusCode );
response.setStatus( code );
response.addHeader( HttpHeaders.CONTENT_TYPE, "application/json; charset=UTF-8" );
addHeaders( response );
response.getOutputStream().write( JsonHelper.createJsonFrom( body() ).getBytes( StandardCharsets.UTF_8 ) );
}
}

private static final ErrorResponse NO_HEADER = new ErrorResponse( 401 )
{
@Override
void addHeaders( HttpServletResponse response )
{
response.addHeader( HttpHeaders.WWW_AUTHENTICATE, "None" );
}

@Override
Object body()
{
return map( "errors", asList( map(
"code", Status.Security.AuthorizationFailed.code().serialize(),
"message", "No authorization header supplied." ) ) );
}
};

private static ErrorResponse noHeader()
{
return NO_HEADER;
}

private static final ErrorResponse BAD_HEADER = new ErrorResponse( 400 )
{
@Override
Object body()
{
return map( "errors", asList( map(
"code", Status.Request.InvalidFormat.code().serialize(),
"message", "Invalid Authorization header." ) ) );
}
};

private static ErrorResponse badHeader()
{
return BAD_HEADER;
response.getOutputStream().write( JsonHelper.createJsonFrom( body ).getBytes( StandardCharsets.UTF_8 ) );
};
}

private static final ErrorResponse INVALID_CREDENTIAL = new ErrorResponse( 401 )
{
@Override
void addHeaders( HttpServletResponse response )
{
response.addHeader( HttpHeaders.WWW_AUTHENTICATE, "None" );
}

@Override
Object body()
{
return map( "errors", asList( map(
"code", Status.Security.AuthorizationFailed.code().serialize(),
"message", "Invalid username or password." ) ) );
}
};

private static ErrorResponse invalidCredential()
{
return INVALID_CREDENTIAL;
private static final ThrowingConsumer<HttpServletResponse, IOException> noHeader =
error( 401,
map( "errors", singletonList( map(
"code", Status.Security.AuthorizationFailed.code().serialize(),
"message", "No authorization header supplied." ) ) ) );

private static final ThrowingConsumer<HttpServletResponse, IOException> badHeader =
error( 400,
map( "errors", singletonList( map(
"code", Status.Request.InvalidFormat.code().serialize(),
"message", "Invalid Authorization header." ) ) ) );

private static final ThrowingConsumer<HttpServletResponse, IOException> invalidCredential =
error( 401,
map( "errors", singletonList( map(
"code", Status.Security.AuthorizationFailed.code().serialize(),
"message", "Invalid username or password." ) ) ) );

private static final ThrowingConsumer<HttpServletResponse, IOException> tooManyAttempts =
error( 429,
map( "errors", singletonList( map(
"code", Status.Security.AuthenticationRateLimit.code().serialize(),
"message", "Too many failed authentication requests. Please wait 5 seconds and try again." ) ) ) );

private static ThrowingConsumer<HttpServletResponse, IOException> passwordChangeRequired( final String username, final String baseURL )
{
URI path = UriBuilder.fromUri( baseURL ).path( format( "/user/%s/password", username ) ).build();
return error( 403,
map( "errors", singletonList( map(
"code", Status.Security.AuthorizationFailed.code().serialize(),
"message", "User is required to change their password." ) ), "password_change", path.toString() ) );
}

private static final ErrorResponse TOO_MANY_ATTEMPTS = new ErrorResponse( 429 )
{
@Override
Object body()
{
return map( "errors", asList( map(
"code", Status.Security.AuthenticationRateLimit.code().serialize(),
"message", "Too many failed authentication requests. Please wait 5 seconds and try again." ) ) );
/**
* In order to avoid browsers popping up an auth box when using the Neo4j Browser, it sends us a special header.
* When we get that special header, we send a crippled authentication challenge back that the browser does not
* understand, which lets the Neo4j Browser handle auth on its own.
*
* Otherwise, we send a regular basic auth challenge. This method adds the appropriate header depending on the
* inbound request.
*/
private static ThrowingConsumer<HttpServletResponse, IOException> requestAuthentication(
HttpServletRequest req, ThrowingConsumer<HttpServletResponse, IOException> responseGen )
{
if( "true".equals( req.getHeader( "X-Ajax-Browser-Auth" ) ) )
{
return (res) -> {
responseGen.accept( res );
res.addHeader( HttpHeaders.WWW_AUTHENTICATE, "None" );
};
} else {
return (res) -> {
responseGen.accept( res );
res.addHeader( HttpHeaders.WWW_AUTHENTICATE, "Basic realm=\"Neo4j\"" );
};
}
};

private static ErrorResponse tooManyAttemptes()
{
return TOO_MANY_ATTEMPTS;
}

private static ErrorResponse passwordChangeRequired( final String username, final String baseURL )
{
return new ErrorResponse( 403 )
{
@Override
Object body()
{
URI path = UriBuilder.fromUri( baseURL ).path( format( "/user/%s/password", username ) ).build();
return map( "errors", asList( map(
"code", Status.Security.AuthorizationFailed.code().serialize(),
"message", "User is required to change their password."
) ), "password_change", path.toString() );
}
};
}

private String baseURL( HttpServletRequest request )
Expand Down
39 changes: 30 additions & 9 deletions community/server/src/test/java/org/neo4j/server/rest/dbms/AuthorizationFilterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,25 @@
import org.apache.commons.codec.binary.Base64;
import org.junit.Before;
import org.junit.Test;
import org.neo4j.logging.AssertableLogProvider;
import org.neo4j.server.security.auth.AuthManager;
import org.neo4j.server.security.auth.AuthenticationResult;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.regex.Pattern;
import javax.servlet.FilterChain;
import javax.servlet.ServletOutputStream;
import javax.servlet.WriteListener;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.ws.rs.core.HttpHeaders;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.regex.Pattern;
import org.neo4j.logging.AssertableLogProvider;
import org.neo4j.server.security.auth.AuthManager;
import org.neo4j.server.security.auth.AuthenticationResult;

import static javax.servlet.http.HttpServletRequest.BASIC_AUTH;
import static org.hamcrest.Matchers.containsString;
import static org.junit.Assert.*;
import static org.junit.Assert.assertThat;
import static org.mockito.Matchers.eq;
import static org.mockito.Matchers.same;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -130,7 +130,7 @@ public void shouldRequireAuthorizationForNonWhitelistedUris() throws Exception
// Then
verifyNoMoreInteractions( filterChain );
verify( servletResponse ).setStatus( 401 );
verify( servletResponse ).addHeader( HttpHeaders.WWW_AUTHENTICATE, "None" );
verify( servletResponse ).addHeader( HttpHeaders.WWW_AUTHENTICATE, "Basic realm=\"Neo4j\"" );
verify( servletResponse ).addHeader( HttpHeaders.CONTENT_TYPE, "application/json; charset=UTF-8" );
assertThat( outputStream.toString( StandardCharsets.UTF_8.name() ), containsString( "\"code\" : \"Neo.ClientError" +".Security.AuthorizationFailed\"" ) );
assertThat( outputStream.toString( StandardCharsets.UTF_8.name() ), containsString( "\"message\" : \"No authorization header supplied.\"" ) );
Expand Down Expand Up @@ -264,4 +264,25 @@ public void shouldAuthorizeWhenValidCredentialsSupplied() throws Exception
// Then
verify( filterChain ).doFilter( eq( new AuthorizedRequestWrapper( BASIC_AUTH, "foo", servletRequest ) ), same( servletResponse ) );
}

@Test
public void shouldIncludeCrippledAuthHeaderIfBrowserIsTheOneCalling() throws Throwable
{
// Given
final AuthorizationFilter filter = new AuthorizationFilter( authManager, logProvider, Pattern.compile( "/" ), Pattern.compile( "/browser.*" ) );
when( servletRequest.getMethod() ).thenReturn( "GET" );
when( servletRequest.getContextPath() ).thenReturn( "/db/data" );
when( servletRequest.getHeader( "X-Ajax-Browser-Auth" )).thenReturn( "true" );

// When
filter.doFilter( servletRequest, servletResponse, filterChain );

// Then
verifyNoMoreInteractions( filterChain );
verify( servletResponse ).setStatus( 401 );
verify( servletResponse ).addHeader( HttpHeaders.WWW_AUTHENTICATE, "None" );
verify( servletResponse ).addHeader( HttpHeaders.CONTENT_TYPE, "application/json; charset=UTF-8" );
assertThat( outputStream.toString( StandardCharsets.UTF_8.name() ), containsString( "\"code\" : \"Neo.ClientError" +".Security.AuthorizationFailed\"" ) );
assertThat( outputStream.toString( StandardCharsets.UTF_8.name() ), containsString( "\"message\" : \"No authorization header supplied.\"" ) );
}
}
8 changes: 4 additions & 4 deletions community/server/src/test/java/org/neo4j/server/rest/security/AuthenticationDocIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void missing_authorization() throws JsonParseException, IOException
RESTDocsGenerator.ResponseEntity response = gen.get()
.noGraph()
.expectedStatus( 401 )
.expectedHeader( "WWW-Authenticate", "None" )
.expectedHeader( "WWW-Authenticate", "Basic realm=\"Neo4j\"" )
.get( dataURL() );

// Then
Expand Down Expand Up @@ -122,7 +122,7 @@ public void incorrect_authentication() throws JsonParseException, IOException
.noGraph()
.expectedStatus( 401 )
.withHeader( HttpHeaders.AUTHORIZATION, challengeResponse( "neo4j", "incorrect" ) )
.expectedHeader( "WWW-Authenticate", "None" )
.expectedHeader( "WWW-Authenticate", "Basic realm=\"Neo4j\"" )
.post( dataURL() );

// Then
Expand Down Expand Up @@ -261,7 +261,7 @@ private void assertAuthorizationRequired( String method, String path, Object pay
assertThat(response.status(), equalTo(401));
assertThat(response.get("errors").get(0).get("code").asText(), equalTo("Neo.ClientError.Security.AuthorizationFailed"));
assertThat(response.get("errors").get(0).get("message").asText(), equalTo("No authorization header supplied."));
assertThat(response.header( HttpHeaders.WWW_AUTHENTICATE ), equalTo("None"));
assertThat(response.header( HttpHeaders.WWW_AUTHENTICATE ), equalTo("Basic realm=\"Neo4j\""));

// When malformed header
response = HTTP.withHeaders( HttpHeaders.AUTHORIZATION, "This makes no sense" ).request( method, server.baseUri().resolve( path ).toString(), payload );
Expand All @@ -274,7 +274,7 @@ private void assertAuthorizationRequired( String method, String path, Object pay
assertThat(response.status(), equalTo(401));
assertThat(response.get("errors").get(0).get("code").asText(), equalTo("Neo.ClientError.Security.AuthorizationFailed"));
assertThat(response.get("errors").get(0).get("message").asText(), equalTo("Invalid username or password."));
assertThat(response.header(HttpHeaders.WWW_AUTHENTICATE ), equalTo("None"));
assertThat(response.header(HttpHeaders.WWW_AUTHENTICATE ), equalTo("Basic realm=\"Neo4j\""));

// When authorized
response = HTTP.withHeaders( HttpHeaders.AUTHORIZATION, challengeResponse( "neo4j", "secret" ) ).request( method, server.baseUri().resolve( path ).toString(), payload );
Expand Down

0 comments on commit c8cccc7

Please sign in to comment.