Skip to content

Commit

Permalink
#8804 Management port returns 403 when no authentication is provided (#…
Browse files Browse the repository at this point in the history
…10819)

(cherry picked from commit 6aa99b2)
  • Loading branch information
rymsha committed Dec 18, 2024
1 parent 903d00a commit c8edb1a
Show file tree
Hide file tree
Showing 7 changed files with 199 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.enonic.xp.portal.idprovider.IdProviderControllerExecutionParams;
import com.enonic.xp.portal.idprovider.IdProviderControllerService;
import com.enonic.xp.security.auth.AuthenticationInfo;
import com.enonic.xp.web.dispatch.DispatchConstants;
import com.enonic.xp.web.filter.OncePerRequestFilter;

@Component(immediate = true, service = Filter.class, property = {"connector=xp", "connector=api"})
Expand Down Expand Up @@ -48,8 +49,12 @@ protected void doHandle( final HttpServletRequest req, final HttpServletResponse
}

//Wraps the response to handle 403 errors
final IdProviderResponseWrapper responseWrapper = new IdProviderResponseWrapper( idProviderControllerService, req, res );
final HttpServletResponse response = DispatchConstants.XP_CONNECTOR.equals( req.getAttribute( DispatchConstants.CONNECTOR_ATTRIBUTE ) )
? new IdProviderResponseWrapper( idProviderControllerService, req, res )
: res;

final IdProviderRequestWrapper requestWrapper = new IdProviderRequestWrapper( req );
chain.doFilter( requestWrapper, responseWrapper );

chain.doFilter( requestWrapper, response );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.io.UncheckedIOException;
import java.io.Writer;

import javax.servlet.ServletOutputStream;
import javax.servlet.WriteListener;
Expand All @@ -13,9 +14,6 @@
import com.enonic.xp.context.ContextAccessor;
import com.enonic.xp.portal.idprovider.IdProviderControllerExecutionParams;
import com.enonic.xp.portal.idprovider.IdProviderControllerService;
import com.enonic.xp.security.auth.AuthenticationInfo;
import com.enonic.xp.util.Exceptions;


public class IdProviderResponseWrapper
extends HttpServletResponseWrapper
Expand All @@ -40,7 +38,14 @@ public IdProviderResponseWrapper( final IdProviderControllerService idProviderCo
@Override
public void setStatus( final int sc )
{
handleError( sc );
try
{
handleError( sc );
}
catch ( IOException e )
{
throw new UncheckedIOException( e );
}

if ( !errorHandled )
{
Expand All @@ -54,7 +59,7 @@ public PrintWriter getWriter()
{
if ( errorHandled )
{
return new PrintWriter( new StringWriter() );
return new PrintWriter( Writer.nullWriter() );
}
return super.getWriter();
}
Expand Down Expand Up @@ -124,43 +129,30 @@ public void sendError( final int sc, final String msg )
}

private void handleError( final int sc )
throws IOException
{
if ( !errorHandled && isUnauthorizedError( sc ) && !isErrorAlreadyHandled() )
{
try
final IdProviderControllerExecutionParams executionParams = IdProviderControllerExecutionParams.create()
.functionName( "handle401" )
.servletRequest( request )
.response( response )
.build();
final boolean responseSerialized = idProviderControllerService.execute( executionParams ) != null;
if ( responseSerialized )
{
final IdProviderControllerExecutionParams executionParams = IdProviderControllerExecutionParams.create().
functionName( "handle401" ).
servletRequest( request ).
response( response ).
build();
final boolean responseSerialized = idProviderControllerService.execute( executionParams ) != null;
if ( responseSerialized )
{
errorHandled = true;
}
}
catch ( IOException e )
{
throw Exceptions.unchecked( e );
errorHandled = true;
}
}
}

private boolean isUnauthorizedError( final int sc )
{
return 401 == sc || ( 403 == sc && !isAuthenticated() );
return 401 == sc || 403 == sc && !ContextAccessor.current().getAuthInfo().isAuthenticated();
}

private boolean isErrorAlreadyHandled()
{
return Boolean.TRUE.equals( request.getAttribute( "error.handled" ) );
}


private boolean isAuthenticated()
{
final AuthenticationInfo authInfo = ContextAccessor.current().getAuthInfo();
return authInfo.isAuthenticated();
}
}
1 change: 1 addition & 0 deletions modules/web/web-impl/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ dependencies {
implementation project( ':core:core-internal' )

testImplementation( testFixtures( project(":web:web-jetty") ) )
testImplementation( testFixtures( project(":core:core-api") ) )
}

jar {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package com.enonic.xp.web.impl.auth;

import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.annotation.WebFilter;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.osgi.service.component.annotations.Component;

import com.google.common.net.HttpHeaders;

import com.enonic.xp.annotation.Order;
import com.enonic.xp.web.filter.OncePerRequestFilter;

@Component(immediate = true, service = Filter.class, property = {"connector=api"})
@Order(-20)
@WebFilter("/*")
public class AuthRequiredFilter
extends OncePerRequestFilter
{
@Override
protected void doHandle( final HttpServletRequest req, final HttpServletResponse res, final FilterChain chain )
throws Exception
{
if ( req.getUserPrincipal() == null )
{
res.addHeader( HttpHeaders.WWW_AUTHENTICATE, "Basic");
res.addHeader( HttpHeaders.WWW_AUTHENTICATE, "Bearer");
res.sendError( HttpServletResponse.SC_UNAUTHORIZED );
return;
}
chain.doFilter( req, res );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ protected void doHandle( final HttpServletRequest req, final HttpServletResponse

private void login( final HttpServletRequest req )
{
final AuthenticationInfo authInfo = ContextAccessor.current().getAuthInfo();
if ( authInfo.isAuthenticated() )
{
return;
}

final String header = req.getHeader( HttpHeaders.AUTHORIZATION );
if ( header == null )
{
Expand All @@ -75,23 +81,23 @@ private static String[] parseHeader( final String header )
return null;
}

final String type = header.substring( 0, 5 ).toUpperCase();
if ( !type.equals( HttpServletRequest.BASIC_AUTH ) )
final String type = header.substring( 0, 5 );
if ( !type.equalsIgnoreCase( HttpServletRequest.BASIC_AUTH ) )
{
return null;
}

final String val = header.substring( 6 );

final String decoded = new String( Base64.getDecoder().decode( val ), StandardCharsets.UTF_8 );
final String[] parts = decoded.split( ":" );

if ( parts.length != 2 )
int pos = decoded.indexOf( ':' );
if ( pos == -1 )
{
return null;
}

return parts;
return new String[]{decoded.substring( 0, pos ), decoded.substring( pos + 1 )};
}

private AuthenticationInfo authenticate( final String user, final String password )
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package com.enonic.xp.web.impl.auth;

import javax.servlet.FilterChain;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

import com.google.common.net.HttpHeaders;

import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@ExtendWith(MockitoExtension.class)
class AuthRequiredFilterTest
{
private AuthRequiredFilter authRequiredFilter;

@Mock
private HttpServletRequest request;

@Mock
private HttpServletResponse response;

@Mock
private FilterChain filterChain;

@BeforeEach
void setUp()
{
authRequiredFilter = new AuthRequiredFilter();
}

@Test
void doHandle_whenUserPrincipalIsNull_sendsUnauthorizedError()
throws Exception
{
when( request.getUserPrincipal() ).thenReturn( null );

authRequiredFilter.doHandle( request, response, filterChain );

verify( response ).addHeader( HttpHeaders.WWW_AUTHENTICATE, "Basic" );
verify( response ).addHeader( HttpHeaders.WWW_AUTHENTICATE, "Bearer" );
verify( response ).sendError( HttpServletResponse.SC_UNAUTHORIZED );
verify( filterChain, never() ).doFilter( request, response );
}

@Test
void doHandle_whenUserPrincipalIsNotNull_proceedsWithFilterChain()
throws Exception
{
when( request.getUserPrincipal() ).thenReturn( () -> "user" );

authRequiredFilter.doHandle( request, response, filterChain );

verify( response, never() ).addHeader( anyString(), anyString() );
verify( response, never() ).sendError( anyInt() );
verify( filterChain ).doFilter( request, response );
}
}
Loading

0 comments on commit c8edb1a

Please sign in to comment.