/admin/tool must return 404 #10861 (#10862)

enonic · Jan 20, 2025 · f0c8b76 · f0c8b76
1 parent 80cc553
commit f0c8b76
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 14 deletions.
diff --git a/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/portal/AdminToolHandler.java b/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/portal/AdminToolHandler.java
@@ -1,5 +1,7 @@
 package com.enonic.xp.admin.impl.portal;
 
+import java.util.regex.Pattern;
+
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
 
@@ -12,6 +14,7 @@
 import com.enonic.xp.portal.handler.WebHandlerHelper;
 import com.enonic.xp.trace.Trace;
 import com.enonic.xp.trace.Tracer;
+import com.enonic.xp.web.WebException;
 import com.enonic.xp.web.WebRequest;
 import com.enonic.xp.web.WebResponse;
 import com.enonic.xp.web.handler.BaseWebHandler;
@@ -22,6 +25,8 @@
 public final class AdminToolHandler
     extends BaseWebHandler
 {
+    private static final Pattern TOOL_CXT_PATTERN = Pattern.compile( "^/admin/([^/]+)/([^/]+)" );
+
     private AdminToolDescriptorService adminToolDescriptorService;
 
     private ControllerScriptFactory controllerScriptFactory;
@@ -43,6 +48,12 @@ protected boolean canHandle( final WebRequest webRequest )
     protected WebResponse doHandle( final WebRequest webRequest, final WebResponse webResponse, final WebHandlerChain webHandlerChain )
         throws Exception
     {
+        final String rawPath = webRequest.getRawPath();
+        if ( !( rawPath.equals( "/admin" ) || rawPath.equals( "/admin/" ) ) && !TOOL_CXT_PATTERN.matcher( rawPath ).find() )
+        {
+            throw WebException.notFound( "Invalid admin tool mount" );
+        }
+
         WebHandlerHelper.checkAdminAccess( webRequest );
 
         PortalRequest portalRequest = (PortalRequest) webRequest;

diff --git a/.../admin/admin-impl/src/test/java/com/enonic/xp/admin/impl/portal/AdminToolHandlerTest.java b/.../admin/admin-impl/src/test/java/com/enonic/xp/admin/impl/portal/AdminToolHandlerTest.java
@@ -16,6 +16,7 @@
 import com.enonic.xp.portal.controller.ControllerScriptFactory;
 import com.enonic.xp.resource.ResourceKey;
 import com.enonic.xp.security.PrincipalKeys;
+import com.enonic.xp.web.HttpStatus;
 import com.enonic.xp.web.WebException;
 import com.enonic.xp.web.WebResponse;
 import com.enonic.xp.web.handler.BaseHandlerTest;
@@ -24,6 +25,9 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class AdminToolHandlerTest
     extends BaseHandlerTest
@@ -47,21 +51,21 @@ public class AdminToolHandlerTest
     public final void setup()
     {
 
-        this.adminToolDescriptorService = Mockito.mock( AdminToolDescriptorService.class );
-        ControllerScript controllerScript = Mockito.mock( ControllerScript.class );
+        this.adminToolDescriptorService = mock( AdminToolDescriptorService.class );
+        ControllerScript controllerScript = mock( ControllerScript.class );
 
         this.portalResponse = PortalResponse.create().build();
-        Mockito.when( controllerScript.execute( Mockito.any( PortalRequest.class ) ) ).thenReturn( this.portalResponse );
+        when( controllerScript.execute( any( PortalRequest.class ) ) ).thenReturn( this.portalResponse );
 
-        final ControllerScriptFactory controllerScriptFactory = Mockito.mock( ControllerScriptFactory.class );
-        Mockito.when( controllerScriptFactory.fromScript( Mockito.any( ResourceKey.class ) ) ).thenReturn( controllerScript );
+        final ControllerScriptFactory controllerScriptFactory = mock( ControllerScriptFactory.class );
+        when( controllerScriptFactory.fromScript( any( ResourceKey.class ) ) ).thenReturn( controllerScript );
 
         this.handler = new AdminToolHandler();
         this.handler.setAdminToolDescriptorService( this.adminToolDescriptorService );
         this.handler.setControllerScriptFactory( controllerScriptFactory );
 
-        this.rawRequest = Mockito.mock( HttpServletRequest.class );
-        Mockito.when( this.rawRequest.isUserInRole( Mockito.anyString() ) ).thenReturn( true );
+        this.rawRequest = mock( HttpServletRequest.class );
+        when( this.rawRequest.isUserInRole( Mockito.anyString() ) ).thenReturn( true );
 
         this.portalRequest = new PortalRequest();
         this.portalRequest.setRawRequest( this.rawRequest );
@@ -73,7 +77,7 @@ public final void setup()
 
         this.webResponse = WebResponse.create().build();
 
-        this.chain = Mockito.mock( WebHandlerChain.class );
+        this.chain = mock( WebHandlerChain.class );
     }
 
     @Test
@@ -87,14 +91,14 @@ public void testCanHandle()
     public void testWithoutPermissions()
     {
         this.portalRequest.setRawPath( "/admin/webapp/tool/1" );
-        Mockito.when( this.rawRequest.isUserInRole( Mockito.anyString() ) ).thenReturn( false );
+        when( this.rawRequest.isUserInRole( Mockito.anyString() ) ).thenReturn( false );
         assertThrows( WebException.class, () -> this.handler.doHandle( this.portalRequest, this.webResponse, this.chain ) );
     }
 
     @Test
     public void testWithNoDescriptor()
     {
-        Mockito.when( this.adminToolDescriptorService.getByKey( Mockito.any( DescriptorKey.class ) ) ).thenReturn( null );
+        when( this.adminToolDescriptorService.getByKey( any( DescriptorKey.class ) ) ).thenReturn( null );
         this.portalRequest.setRawPath( "/admin/webapp/tool/1" );
         assertThrows( WebException.class, () -> this.handler.doHandle( this.portalRequest, this.webResponse, this.chain ) );
     }
@@ -119,11 +123,27 @@ public void test()
         assertEquals( "/admin/webapp/tool", this.portalRequest.getContextPath() );
     }
 
+    @Test
+    void testInvalidAdminToolMount()
+    {
+        this.portalRequest.setBaseUri( "/admin" );
+        this.portalRequest.setRawPath( "/admin/tool" );
+        WebException ex =
+            assertThrows( WebException.class, () -> this.handler.doHandle( this.portalRequest, this.webResponse, this.chain ) );
+        assertEquals( HttpStatus.NOT_FOUND, ex.getStatus() );
+        assertEquals( "Invalid admin tool mount", ex.getMessage() );
+
+        this.portalRequest.setRawPath( "/admin/tool/" );
+        ex = assertThrows( WebException.class, () -> this.handler.doHandle( this.portalRequest, this.webResponse, this.chain ) );
+        assertEquals( HttpStatus.NOT_FOUND, ex.getStatus() );
+        assertEquals( "Invalid admin tool mount", ex.getMessage() );
+    }
+
     private void mockDescriptor( DescriptorKey descriptorKey, boolean hasAccess )
     {
-        AdminToolDescriptor descriptor = Mockito.mock( AdminToolDescriptor.class );
-        Mockito.when( descriptor.getKey() ).thenReturn( descriptorKey );
-        Mockito.when( descriptor.isAccessAllowed( Mockito.any( PrincipalKeys.class ) ) ).thenReturn( hasAccess );
-        Mockito.when( this.adminToolDescriptorService.getByKey( Mockito.any( DescriptorKey.class ) ) ).thenReturn( descriptor );
+        AdminToolDescriptor descriptor = mock( AdminToolDescriptor.class );
+        when( descriptor.getKey() ).thenReturn( descriptorKey );
+        when( descriptor.isAccessAllowed( any( PrincipalKeys.class ) ) ).thenReturn( hasAccess );
+        when( this.adminToolDescriptorService.getByKey( any( DescriptorKey.class ) ) ).thenReturn( descriptor );
     }
 }