/admin/tool must return 404 #10861

enonic · Jan 20, 2025 · f67caa6 · f67caa6
1 parent 5167825
commit f67caa6
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 74 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 );
     }
 }
diff --git a/modules/portal/portal-impl/src/main/java/com/enonic/xp/portal/impl/url/PortalUrlBuilder.java b/modules/portal/portal-impl/src/main/java/com/enonic/xp/portal/impl/url/PortalUrlBuilder.java
@@ -23,8 +23,6 @@
 import com.enonic.xp.portal.url.UrlTypeConstants;
 import com.enonic.xp.repository.RepositoryUtils;
 import com.enonic.xp.resource.ResourceService;
-import com.enonic.xp.web.HttpStatus;
-import com.enonic.xp.web.WebException;
 import com.enonic.xp.web.servlet.ServletRequestUrlHelper;
 import com.enonic.xp.web.servlet.UriRewritingResult;
 
@@ -69,17 +67,6 @@ public final String build()
         {
             return doBuild();
         }
-        catch ( WebException e )
-        {
-            if ( e.getStatus() == HttpStatus.NOT_FOUND )
-            {
-                throw e;
-            }
-            else
-            {
-                return buildErrorUrl( e );
-            }
-        }
         catch ( final Exception e )
         {
             return buildErrorUrl( e );

diff --git a/...ortal/portal-impl/src/main/java/com/enonic/xp/portal/impl/url/UniversalApiUrlBuilder.java b/...ortal/portal-impl/src/main/java/com/enonic/xp/portal/impl/url/UniversalApiUrlBuilder.java
@@ -10,8 +10,6 @@
 import com.enonic.xp.portal.impl.ContentResolverResult;
 import com.enonic.xp.portal.url.ApiUrlParams;
 import com.enonic.xp.site.Site;
-import com.enonic.xp.web.HttpStatus;
-import com.enonic.xp.web.WebException;
 
 import static com.enonic.xp.portal.impl.url.UrlBuilderHelper.appendPathSegments;
 import static com.enonic.xp.portal.impl.url.UrlBuilderHelper.appendSubPath;
@@ -47,36 +45,29 @@ protected void buildUrl( final StringBuilder url, final Multimap<String, String>
 
         final String requestURI = this.portalRequest.getRawRequest().getRequestURI();
 
-        try
+        if ( requestURI.equals( ADMIN_PREFIX ) )
         {
-            if ( requestURI.equals( ADMIN_PREFIX ) )
-            {
-                processHome( url );
-            }
-            else if ( requestURI.startsWith( ADMIN_SITE_PREFIX ) )
-            {
-                processSite( url, requestURI, true );
-            }
-            else if ( requestURI.startsWith( TOOL_PREFIX ) )
-            {
-                processTool( url, requestURI );
-            }
-            else if ( requestURI.startsWith( SITE_PREFIX ) )
-            {
-                processSite( url, requestURI, false );
-            }
-            else if ( requestURI.startsWith( WEBAPP_PREFIX ) )
-            {
-                processWebapp( url, requestURI );
-            }
-            else
-            {
-                appendPart( url, "api" );
-            }
+            processHome( url );
+        }
+        else if ( requestURI.startsWith( ADMIN_SITE_PREFIX ) )
+        {
+            processSite( url, requestURI, true );
+        }
+        else if ( requestURI.startsWith( TOOL_PREFIX ) )
+        {
+            processTool( url, requestURI );
         }
-        catch ( IllegalArgumentException e )
+        else if ( requestURI.startsWith( SITE_PREFIX ) )
+        {
+            processSite( url, requestURI, false );
+        }
+        else if ( requestURI.startsWith( WEBAPP_PREFIX ) )
+        {
+            processWebapp( url, requestURI );
+        }
+        else
         {
-            throw new WebException( HttpStatus.NOT_FOUND, "Mount not found", e );
+            appendPart( url, "api" );
         }
 
         if ( !requestURI.startsWith( API_PREFIX ) )

diff --git a/...tal-impl/src/test/java/com/enonic/xp/portal/impl/url/PortalUrlServiceImpl_apiUrlTest.java b/...tal-impl/src/test/java/com/enonic/xp/portal/impl/url/PortalUrlServiceImpl_apiUrlTest.java
@@ -7,11 +7,8 @@
 import com.enonic.xp.content.ContentPath;
 import com.enonic.xp.portal.url.ApiUrlParams;
 import com.enonic.xp.site.Site;
-import com.enonic.xp.web.HttpStatus;
-import com.enonic.xp.web.WebException;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -70,21 +67,6 @@ void testCreateUrlAdminToolWithAppFromRequest()
         assertEquals( "/admin/myapplication/toolname/_/com.enonic.app.myapp:myapi", url );
     }
 
-    @Test
-    void testCreateUrlMountNotFound()
-    {
-        when( portalRequest.getRawRequest().getRequestURI() ).thenReturn( "/admin/toolname" );
-
-        final ApiUrlParams params = new ApiUrlParams();
-        params.portalRequest( this.portalRequest );
-        params.application( "com.enonic.app.myapp" );
-        params.api( "myapi" );
-
-        WebException ex = assertThrows( WebException.class, () -> this.service.apiUrl( params ) );
-        assertEquals( "Mount not found", ex.getMessage() );
-        assertEquals( HttpStatus.NOT_FOUND, ex.getStatus() );
-    }
-
     @Test
     void testCreateUrlAdminSite()
     {