WIP Do not create session for login url #10460

modules/portal/portal-impl/src/main/java/com/enonic/xp/portal/impl/RedirectChecksumGenerator.java

@@ -0,0 +1,67 @@
+package com.enonic.xp.portal.impl;
+
+import java.nio.charset.StandardCharsets;
+import java.security.InvalidKeyException;
+import java.security.NoSuchAlgorithmException;
+
+import javax.crypto.KeyGenerator;
+import javax.crypto.Mac;
+import javax.crypto.SecretKey;
+import javax.crypto.spec.SecretKeySpec;
+
+import org.osgi.service.component.annotations.Activate;
+import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Reference;
+
+import com.enonic.xp.core.internal.HexCoder;
+import com.enonic.xp.shared.SharedMap;
+import com.enonic.xp.shared.SharedMapService;
+
+@Component(service = RedirectChecksumGenerator.class)
+public class RedirectChecksumGenerator
+{
+    private final SharedMap<String, Object> sharedMap;
+
+    @Activate
+    public RedirectChecksumGenerator( @Reference final SharedMapService sharedMapService )
+    {
+        this.sharedMap = sharedMapService.getSharedMap( "com.enonic.xp.internal.shared" );
+    }
+
+    public String generateChecksum( final String redirect )
+    {
+        final SecretKey key = new SecretKeySpec( (byte[]) sharedMap.modify( "redirectTicketKey", orig -> {
+
+            if ( orig != null )
+            {
+                return orig;
+            }
+
+            final KeyGenerator keyGenerator;
+            try
+            {
+                keyGenerator = KeyGenerator.getInstance( "HmacSHA1" );
+            }
+            catch ( NoSuchAlgorithmException e )
+            {
+                throw new IllegalStateException( e );
+            }
+            return keyGenerator.generateKey().getEncoded();
+
+        } ), "HmacSHA1" );
+
+        final Mac mac;
+        try
+        {
+            mac = Mac.getInstance( key.getAlgorithm() );
+
+            mac.init( key );
+        }
+        catch ( NoSuchAlgorithmException | InvalidKeyException e )
+        {
+            throw new RuntimeException( e );
+        }
+
+        return HexCoder.toHex( mac.doFinal( redirect.getBytes( StandardCharsets.UTF_8 ) ) );
+    }
+}

modules/portal/portal-impl/src/main/java/com/enonic/xp/portal/impl/handler/identity/IdentityHandler.java

@@ -1,6 +1,7 @@
 package com.enonic.xp.portal.impl.handler.identity;
 
 import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
 import java.util.Collection;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -9,15 +10,14 @@
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
 
-import com.google.common.hash.Hashing;
-
 import com.enonic.xp.content.ContentService;
 import com.enonic.xp.context.ContextAccessor;
 import com.enonic.xp.portal.PortalRequest;
 import com.enonic.xp.portal.PortalResponse;
 import com.enonic.xp.portal.handler.EndpointHandler;
 import com.enonic.xp.portal.idprovider.IdProviderControllerService;
 import com.enonic.xp.portal.impl.ContentResolver;
+import com.enonic.xp.portal.impl.RedirectChecksumGenerator;
 import com.enonic.xp.security.IdProviderKey;
 import com.enonic.xp.trace.Trace;
 import com.enonic.xp.trace.Tracer;
@@ -41,13 +41,17 @@ public class IdentityHandler
 
     private final IdProviderControllerService idProviderControllerService;
 
+    private final RedirectChecksumGenerator redirectChecksumGenerator;
+
     @Activate
     public IdentityHandler( @Reference final ContentService contentService,
-                            @Reference final IdProviderControllerService idProviderControllerService )
+                            @Reference final IdProviderControllerService idProviderControllerService,
+                            @Reference final RedirectChecksumGenerator redirectChecksumGenerator )
     {
-        super("idprovider");
+        super( "idprovider" );
         this.contentService = contentService;
         this.idProviderControllerService = idProviderControllerService;
+        this.redirectChecksumGenerator = redirectChecksumGenerator;
     }
 
     @Override
@@ -84,15 +88,13 @@ protected PortalResponse doHandle( final WebRequest webRequest, final WebRespons
 
         if ( idProviderFunction == null )
         {
-            idProviderFunction = webRequest.getMethod().
-                toString().
-                toLowerCase();
+            idProviderFunction = webRequest.getMethod().toString().toLowerCase();
         }
 
         final IdentityHandlerWorker worker = new IdentityHandlerWorker( portalRequest );
         worker.idProviderKey = idProviderKey;
         worker.idProviderFunction = idProviderFunction;
-        worker.contentResolver =  new ContentResolver( contentService );
+        worker.contentResolver = new ContentResolver( contentService );
         worker.idProviderControllerService = this.idProviderControllerService;
         final Trace trace = Tracer.newTrace( "portalRequest" );
         if ( trace == null )
@@ -116,24 +118,18 @@ protected PortalResponse doHandle( final WebRequest webRequest, final WebRespons
 
     private void checkTicket( final PortalRequest req )
     {
-        if ( getParameter( req, "redirect" ) != null )
+        final String redirect = getParameter( req, "redirect" );
+        if ( redirect != null )
         {
             final String ticket = removeParameter( req, "_ticket" );
             if ( ticket == null )
             {
                 throw WebException.badRequest( "Missing ticket parameter" );
             }
 
-            final String jSessionId = getJSessionId();
-            final String expectedTicket = generateTicket( jSessionId );
-            if ( expectedTicket.equals( ticket ) )
-            {
-                req.setValidTicket( Boolean.TRUE );
-            }
-            else
-            {
-                req.setValidTicket( Boolean.FALSE );
-            }
+            final String expectedTicket = redirectChecksumGenerator.generateChecksum( redirect );
+            req.setValidTicket( MessageDigest.isEqual( expectedTicket.getBytes( StandardCharsets.ISO_8859_1 ),
+                                                       ticket.getBytes( StandardCharsets.ISO_8859_1 ) ) );
         }
     }
 
@@ -148,22 +144,4 @@ private String removeParameter( final PortalRequest req, final String name )
         final Collection<String> values = req.getParams().removeAll( name );
         return values.isEmpty() ? null : values.iterator().next();
     }
-
-    private String getJSessionId()
-    {
-        return ContextAccessor.current().
-            getLocalScope().
-            getSession().
-            getKey().
-            toString();
-    }
-
-    private String generateTicket( final String jSessionId )
-    {
-        return Hashing.sha1().
-            newHasher().
-            putString( jSessionId, StandardCharsets.UTF_8 ).
-            hash().
-            toString();
-    }
 }

modules/portal/portal-impl/src/main/java/com/enonic/xp/portal/impl/url/IdentityUrlBuilder.java

@@ -1,20 +1,20 @@
 package com.enonic.xp.portal.impl.url;
 
-import java.nio.charset.StandardCharsets;
+import java.util.function.Function;
 
 import com.google.common.collect.Multimap;
-import com.google.common.hash.Hashing;
 
-import com.enonic.xp.context.ContextAccessor;
 import com.enonic.xp.portal.url.IdentityUrlParams;
 
 final class IdentityUrlBuilder
     extends GenericEndpointUrlBuilder<IdentityUrlParams>
 {
 
-    IdentityUrlBuilder()
+    private final Function<String, String> checksumGenerator;
+    IdentityUrlBuilder( Function<String, String> checksumGenerator )
     {
         super( "idprovider" );
+        this.checksumGenerator = checksumGenerator;
     }
 
     @Override
@@ -47,8 +47,7 @@ protected void buildUrl( final StringBuilder url, final Multimap<String, String>
         {
             params.put( "redirect", this.params.getRedirectionUrl() );
 
-            final String jSessionId = getJSessionId();
-            params.put( "_ticket", generateTicket( jSessionId ) );
+            params.put( "_ticket", checksumGenerator.apply( redirectionUrl ) );
         }
     }
 
@@ -64,21 +63,4 @@ protected String getTargetUriPrefix()
         return "/api/idprovider";
     }
 
-    private String getJSessionId()
-    {
-        return ContextAccessor.current().
-            getLocalScope().
-            getSession().
-            getKey().
-            toString();
-    }
-
-    private String generateTicket( final String jSessionId )
-    {
-        return Hashing.sha1().
-            newHasher().
-            putString( jSessionId, StandardCharsets.UTF_8 ).
-            hash().
-            toString();
-    }
 }

modules/portal/portal-impl/src/main/java/com/enonic/xp/portal/impl/url/PortalUrlServiceImpl.java

@@ -2,6 +2,7 @@
 
 import java.util.concurrent.Callable;
 
+import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
 
@@ -10,6 +11,7 @@
 import com.enonic.xp.context.ContextAccessor;
 import com.enonic.xp.context.ContextBuilder;
 import com.enonic.xp.macro.MacroService;
+import com.enonic.xp.portal.impl.RedirectChecksumGenerator;
 import com.enonic.xp.portal.url.AbstractUrlParams;
 import com.enonic.xp.portal.url.AssetUrlParams;
 import com.enonic.xp.portal.url.AttachmentUrlParams;
@@ -30,13 +32,27 @@
 public final class PortalUrlServiceImpl
     implements PortalUrlService
 {
-    private ContentService contentService;
+    private final ContentService contentService;
 
-    private ResourceService resourceService;
+    private final ResourceService resourceService;
 
-    private MacroService macroService;
+    private final MacroService macroService;
 
-    private StyleDescriptorService styleDescriptorService;
+    private final StyleDescriptorService styleDescriptorService;
+
+    private final RedirectChecksumGenerator redirectChecksumGenerator;
+
+    @Activate
+    public PortalUrlServiceImpl( @Reference final ContentService contentService, @Reference final ResourceService resourceService,
+                                 @Reference final MacroService macroService, @Reference final StyleDescriptorService styleDescriptorService,
+                                 @Reference final RedirectChecksumGenerator redirectChecksumGenerator )
+    {
+        this.contentService = contentService;
+        this.resourceService = resourceService;
+        this.macroService = macroService;
+        this.styleDescriptorService = styleDescriptorService;
+        this.redirectChecksumGenerator = redirectChecksumGenerator;
+    }
 
     @Override
     public String assetUrl( final AssetUrlParams params )
@@ -77,7 +93,7 @@ public String attachmentUrl( final AttachmentUrlParams params )
     @Override
     public String identityUrl( final IdentityUrlParams params )
     {
-        return build( new IdentityUrlBuilder(), params );
+        return build( new IdentityUrlBuilder(redirectChecksumGenerator::generateChecksum), params );
     }
 
     @Override
@@ -100,30 +116,6 @@ private <B extends PortalUrlBuilder<P>, P extends AbstractUrlParams> String buil
         return runWithAdminRole( builder::build );
     }
 
-    @Reference
-    public void setContentService( final ContentService contentService )
-    {
-        this.contentService = contentService;
-    }
-
-    @Reference
-    public void setResourceService( final ResourceService resourceService )
-    {
-        this.resourceService = resourceService;
-    }
-
-    @Reference
-    public void setStyleDescriptorService( final StyleDescriptorService styleDescriptorService )
-    {
-        this.styleDescriptorService = styleDescriptorService;
-    }
-
-    @Reference
-    public void setMacroService( final MacroService macroService )
-    {
-        this.macroService = macroService;
-    }
-
     private <T> T runWithAdminRole( final Callable<T> callable )
     {
         final Context context = ContextAccessor.current();

modules/portal/portal-impl/src/test/java/com/enonic/xp/portal/impl/handler/identity/IdentityHandlerTest.java

@@ -25,6 +25,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.fail;
+import static org.mockito.Mockito.mock;
 
 public class IdentityHandlerTest
     extends BaseHandlerTest
@@ -40,9 +41,9 @@ public final void setup()
         throws Exception
     {
         this.request = new PortalRequest();
-        final ContentService contentService = Mockito.mock( ContentService.class );
-        final IdProviderControllerService idProviderControllerService = Mockito.mock( IdProviderControllerService.class );
-        final HttpServletRequest rawRequest = Mockito.mock( HttpServletRequest.class );
+        final ContentService contentService = mock( ContentService.class );
+        final IdProviderControllerService idProviderControllerService = mock( IdProviderControllerService.class );
+        final HttpServletRequest rawRequest = mock( HttpServletRequest.class );
 
         Mockito.when( idProviderControllerService.execute( Mockito.any() ) ).thenAnswer( invocation -> {
             Object[] args = invocation.getArguments();
@@ -54,7 +55,7 @@ public final void setup()
             return null;
         } );
 
-        this.handler = new IdentityHandler( contentService, idProviderControllerService );
+        this.handler = new IdentityHandler( contentService, idProviderControllerService, mock() );
 
         this.request.setMethod( HttpMethod.GET );
         this.request.setEndpointPath( "/_/idprovider/myidprovider?param1=value1" );
@@ -88,10 +89,10 @@ public void testMatch()
     public void testOptions()
         throws Exception
     {
-        final IdProviderControllerService idProviderControllerService = Mockito.mock( IdProviderControllerService.class );
+        final IdProviderControllerService idProviderControllerService = mock( IdProviderControllerService.class );
         final PortalResponse response = PortalResponse.create().status( HttpStatus.METHOD_NOT_ALLOWED ).build();
         Mockito.when( idProviderControllerService.execute( Mockito.any() ) ).thenReturn( response );
-        this.handler = new IdentityHandler( Mockito.mock( ContentService.class ), idProviderControllerService );
+        this.handler = new IdentityHandler( mock( ContentService.class ), idProviderControllerService, mock() );
 
         this.request.setMethod( HttpMethod.OPTIONS );
 
@@ -136,7 +137,7 @@ public void testHandleWithVirtualHostNotEnabled()
     {
         final HttpServletRequest rawRequest = this.request.getRawRequest();
 
-        final VirtualHost virtualHost = Mockito.mock( VirtualHost.class );
+        final VirtualHost virtualHost = mock( VirtualHost.class );
         Mockito.when( virtualHost.getIdProviderKeys() ).thenReturn( IdProviderKeys.from( "otherEnabledIdProvider" ) );
 
         VirtualHostHelper.setVirtualHost( rawRequest, initVirtualHost( rawRequest, virtualHost ) );
@@ -157,7 +158,7 @@ public void testHandleWithVirtualHostEnabled()
     {
         final HttpServletRequest rawRequest = this.request.getRawRequest();
 
-        final VirtualHost virtualHost = Mockito.mock( VirtualHost.class );
+        final VirtualHost virtualHost = mock( VirtualHost.class );
         Mockito.when( virtualHost.getIdProviderKeys() ).thenReturn( IdProviderKeys.from( "otherEnabledIdProvider", "myidprovider" ) );
 
         VirtualHostHelper.setVirtualHost( rawRequest, initVirtualHost( rawRequest, virtualHost ) );
@@ -175,7 +176,7 @@ public void testHandleWithEmptyVirtualHostIdProviderConfig()
     {
         final HttpServletRequest rawRequest = this.request.getRawRequest();
 
-        final VirtualHost virtualHost = Mockito.mock( VirtualHost.class );
+        final VirtualHost virtualHost = mock( VirtualHost.class );
         Mockito.when( virtualHost.getIdProviderKeys() ).thenReturn( IdProviderKeys.empty() );
 
         VirtualHostHelper.setVirtualHost( rawRequest, virtualHost );