WIP Do not create session for login url #10460

modules/core/core-api/src/main/java/com/enonic/xp/security/SecurityService.java

@@ -17,6 +17,7 @@ public interface SecurityService
 
     IdProviderAccessControlList getIdProviderPermissions( IdProviderKey idProviderKey );
 
+    @Deprecated
     IdProviderAccessControlList getDefaultIdProviderPermissions();
 
     IdProvider createIdProvider( CreateIdProviderParams createIdProviderParams );

modules/core/core-security/src/main/java/com/enonic/xp/core/impl/security/IdProviderNodeTranslator.java

@@ -44,20 +44,9 @@ abstract class IdProviderNodeTranslator
 
     static final String GROUP_FOLDER_NODE_NAME = "groups";
 
-    private static final ApplicationKey SYSTEM_ID_PROVIDER_KEY = ApplicationKey.from( "com.enonic.xp.app.standardidprovider" );
+    static final ApplicationKey SYSTEM_ID_PROVIDER_KEY = ApplicationKey.from( "com.enonic.xp.app.standardidprovider" );
 
-    protected static final NodePath ID_PROVIDER_PARENT_PATH =
-        new NodePath( NodePath.ROOT, NodeName.from( PrincipalKey.IDENTITY_NODE_NAME ) );
-
-    static NodePath getRolesNodePath()
-    {
-        return new NodePath( ID_PROVIDER_PARENT_PATH, NodeName.from( PrincipalKey.ROLES_NODE_NAME ) );
-    }
-
-    static NodePath getIdProvidersParentPath()
-    {
-        return ID_PROVIDER_PARENT_PATH;
-    }
+    static final NodePath ID_PROVIDERS_PARENT_PATH = new NodePath( NodePath.ROOT, NodeName.from( PrincipalKey.IDENTITY_NODE_NAME ) );
 
     static IdProviderKey toKey( final Node node )
     {
@@ -67,7 +56,7 @@ static IdProviderKey toKey( final Node node )
 
     static NodePath toIdProviderNodePath( final IdProviderKey idProviderKey )
     {
-        return new NodePath( ID_PROVIDER_PARENT_PATH, NodeName.from( idProviderKey.toString() ) );
+        return new NodePath( ID_PROVIDERS_PARENT_PATH, NodeName.from( idProviderKey.toString() ) );
     }
 
     static NodePath toIdProviderUsersNodePath( final IdProviderKey idProviderKey )

modules/core/core-security/src/main/java/com/enonic/xp/core/impl/security/SecurityServiceImpl.java

@@ -151,7 +151,7 @@ private void initializeSuPassword()
     public IdProviders getIdProviders()
     {
         final FindNodesByParentParams findByParent =
-            FindNodesByParentParams.create().parentPath( IdProviderNodeTranslator.getIdProvidersParentPath() ).build();
+            FindNodesByParentParams.create().parentPath( IdProviderNodeTranslator.ID_PROVIDERS_PARENT_PATH ).build();
         final Nodes nodes = callWithContext( () -> {
             final FindNodesByParentResult result = this.nodeService.findByParent( findByParent );
             return this.nodeService.getByIds( result.getNodeIds() );
@@ -371,7 +371,7 @@ private boolean isSuAuthenticationEnabled( final AuthenticationToken token )
             UsernamePasswordAuthToken usernamePasswordAuthToken = (UsernamePasswordAuthToken) token;
             return ( usernamePasswordAuthToken.getIdProvider() == null ||
                 IdProviderKey.system().equals( usernamePasswordAuthToken.getIdProvider() ) ) &&
-                SecurityInitializer.SUPER_USER.getId().equals( usernamePasswordAuthToken.getUsername() );
+                PrincipalKey.ofSuperUser().getId().equals( usernamePasswordAuthToken.getUsername() );
         }
         return false;
     }
@@ -382,8 +382,8 @@ private AuthenticationInfo authenticateSu( final UsernamePasswordAuthToken token
         if ( this.suPasswordValue.equals( hashedTokenPassword ) )
         {
             final User admin = User.create()
-                .key( SecurityInitializer.SUPER_USER )
-                .login( SecurityInitializer.SUPER_USER.getId() )
+                .key( PrincipalKey.ofSuperUser() )
+                .login( PrincipalKey.ofSuperUser().getId() )
                 .displayName( "Super User" )
                 .build();
             return AuthenticationInfo.create()
@@ -969,7 +969,7 @@ public IdProvider createIdProvider( final CreateIdProviderParams createIdProvide
                 groupsNodePermissions = mergeWithRootPermissions( groupsNodePermissions, rootNode.getPermissions() );
 
                 final Node idProviderNode = nodeService.create( CreateNodeParams.create()
-                                                                    .parent( IdProviderNodeTranslator.getIdProvidersParentPath() )
+                                                                    .parent( IdProviderNodeTranslator.ID_PROVIDERS_PARENT_PATH )
                                                                     .name( createIdProviderParams.getKey().toString() )
                                                                     .data( data )
                                                                     .permissions( idProviderNodePermissions )

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

@@ -0,0 +1,93 @@
+package com.enonic.xp.portal.impl;
+
+import java.nio.charset.StandardCharsets;
+import java.security.InvalidKeyException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.util.Arrays;
+import java.util.Base64;
+import java.util.function.Supplier;
+
+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.google.common.base.Suppliers;
+
+import com.enonic.xp.context.Context;
+import com.enonic.xp.context.ContextAccessor;
+import com.enonic.xp.context.ContextBuilder;
+import com.enonic.xp.core.internal.HexCoder;
+import com.enonic.xp.node.NodePath;
+import com.enonic.xp.node.NodeService;
+import com.enonic.xp.security.RoleKeys;
+import com.enonic.xp.security.SecurityService;
+import com.enonic.xp.security.SystemConstants;
+import com.enonic.xp.security.auth.AuthenticationInfo;
+
+@Component(service = RedirectChecksumService.class)
+public class RedirectChecksumService
+{
+    private static final NodePath GENERIC_KEY_PATH = NodePath.create().addElement( "keys" ).addElement( "generic-hmac-sha512" ).build();
+
+    private final NodeService nodeService;
+
+    /**
+     * Used to make sure the SecurityInitializer is run before this component is activated.
+     */
+    @SuppressWarnings("unused")
+    @Reference
+    private SecurityService securityService;
+
+    private final Supplier<SecretKey> keySupplier = Suppliers.memoize( this::doGetKey );
+
+    @Activate
+    public RedirectChecksumService( @Reference final NodeService nodeService )
+    {
+        this.nodeService = nodeService;
+    }
+
+    private SecretKey doGetKey()
+    {
+        final String storedKey =
+            createSystemContext().callWith( () -> nodeService.getByPath( GENERIC_KEY_PATH ) ).data().getString( "key" );
+        return new SecretKeySpec( Base64.getDecoder().decode( storedKey ), "HmacSHA512" );
+    }
+
+    public String generateChecksum( final String redirect )
+    {
+        final SecretKey key = keySupplier.get();
+        final Mac mac;
+        try
+        {
+            mac = Mac.getInstance( key.getAlgorithm() );
+            mac.init( key );
+        }
+        catch ( NoSuchAlgorithmException | InvalidKeyException e )
+        {
+            throw new IllegalStateException( e );
+        }
+
+        return HexCoder.toHex( Arrays.copyOf( mac.doFinal( redirect.getBytes( StandardCharsets.UTF_8 ) ), 20 ) );
+    }
+
+    public boolean verifyChecksum( final String redirect, final String checksum )
+    {
+        final String expectedTicket = generateChecksum( redirect );
+        return MessageDigest.isEqual( expectedTicket.getBytes( StandardCharsets.ISO_8859_1 ),
+                                      checksum.getBytes( StandardCharsets.ISO_8859_1 ) );
+    }
+
+    private static Context createSystemContext()
+    {
+        return ContextBuilder.from( ContextAccessor.current() )
+            .authInfo( AuthenticationInfo.copyOf( ContextAccessor.current().getAuthInfo() ).principals( RoleKeys.ADMIN ).build() )
+            .repositoryId( SystemConstants.SYSTEM_REPO_ID )
+            .branch( SystemConstants.BRANCH_SYSTEM )
+            .build();
+    }
+}

modules/portal/portal-impl/src/main/java/com/enonic/xp/portal/impl/handler/api/ApiIdentityHandler.java

@@ -1,7 +1,6 @@
 package com.enonic.xp.portal.impl.handler.api;
 
 import java.io.IOException;
-import java.nio.charset.StandardCharsets;
 import java.util.Collection;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -10,13 +9,12 @@
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
 
-import com.google.common.hash.Hashing;
-
 import com.enonic.xp.context.ContextAccessor;
 import com.enonic.xp.portal.PortalRequest;
 import com.enonic.xp.portal.PortalResponse;
 import com.enonic.xp.portal.idprovider.IdProviderControllerExecutionParams;
 import com.enonic.xp.portal.idprovider.IdProviderControllerService;
+import com.enonic.xp.portal.impl.RedirectChecksumService;
 import com.enonic.xp.security.IdProviderKey;
 import com.enonic.xp.trace.Trace;
 import com.enonic.xp.trace.Tracer;
@@ -39,10 +37,14 @@ public class ApiIdentityHandler
 
     private final IdProviderControllerService idProviderControllerService;
 
+    private final RedirectChecksumService redirectChecksumService;
+
     @Activate
-    public ApiIdentityHandler( @Reference final IdProviderControllerService idProviderControllerService )
+    public ApiIdentityHandler( @Reference final IdProviderControllerService idProviderControllerService,
+                               @Reference final RedirectChecksumService redirectChecksumService )
     {
         this.idProviderControllerService = idProviderControllerService;
+        this.redirectChecksumService = redirectChecksumService;
     }
 
     @Override
@@ -145,17 +147,16 @@ private PortalResponse doExecute( final IdProviderKey idProviderKey, final Strin
 
     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 );
-            req.setValidTicket( expectedTicket.equals( ticket ) );
+            req.setValidTicket( redirectChecksumService.verifyChecksum( redirect, ticket ) );
         }
     }
 
@@ -170,14 +171,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/handler/identity/IdentityHandler.java

@@ -1,6 +1,5 @@
 package com.enonic.xp.portal.impl.handler.identity;
 
-import java.nio.charset.StandardCharsets;
 import java.util.Collection;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -9,15 +8,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.RedirectChecksumService;
 import com.enonic.xp.security.IdProviderKey;
 import com.enonic.xp.trace.Trace;
 import com.enonic.xp.trace.Tracer;
@@ -41,13 +39,17 @@ public class IdentityHandler
 
     private final IdProviderControllerService idProviderControllerService;
 
+    private final RedirectChecksumService redirectChecksumService;
+
     @Activate
     public IdentityHandler( @Reference final ContentService contentService,
-                            @Reference final IdProviderControllerService idProviderControllerService )
+                            @Reference final IdProviderControllerService idProviderControllerService,
+                            @Reference final RedirectChecksumService redirectChecksumService )
     {
-        super("idprovider");
+        super( "idprovider" );
         this.contentService = contentService;
         this.idProviderControllerService = idProviderControllerService;
+        this.redirectChecksumService = redirectChecksumService;
     }
 
     @Override
@@ -84,15 +86,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 +116,16 @@ 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 );
-            }
+            req.setValidTicket( redirectChecksumService.verifyChecksum( redirect, ticket ) );
         }
     }
 
@@ -148,22 +140,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();
-    }
 }