Skip to content

Commit

Permalink
Do not create session for login url #10460
Browse files Browse the repository at this point in the history
  • Loading branch information
rymsha committed Apr 4, 2024
1 parent 55fed48 commit e2b88f1
Show file tree
Hide file tree
Showing 15 changed files with 534 additions and 329 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public interface SecurityService

IdProviderAccessControlList getIdProviderPermissions( IdProviderKey idProviderKey );

@Deprecated
IdProviderAccessControlList getDefaultIdProviderPermissions();

IdProvider createIdProvider( CreateIdProviderParams createIdProviderParams );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
{
Expand All @@ -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 )
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -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();

Check warning on line 154 in modules/core/core-security/src/main/java/com/enonic/xp/core/impl/security/SecurityServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L154 was not covered by tests
final Nodes nodes = callWithContext( () -> {
final FindNodesByParentResult result = this.nodeService.findByParent( findByParent );
return this.nodeService.getByIds( result.getNodeIds() );
Expand Down Expand Up @@ -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;
}
Expand All @@ -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() )

Check warning on line 386 in modules/core/core-security/src/main/java/com/enonic/xp/core/impl/security/SecurityServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

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

Added lines #L385 - L386 were not covered by tests
.displayName( "Super User" )
.build();
return AuthenticationInfo.create()
Expand Down Expand Up @@ -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 )
Expand Down
Original file line number Diff line number Diff line change
@@ -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 )

Check warning on line 70 in modules/portal/portal-impl/src/main/java/com/enonic/xp/portal/impl/RedirectChecksumService.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L70 was not covered by tests
{
throw new IllegalStateException( e );

Check warning on line 72 in modules/portal/portal-impl/src/main/java/com/enonic/xp/portal/impl/RedirectChecksumService.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L72 was not covered by tests
}

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();
}
}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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 ) );
}
}

Expand All @@ -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();
}
}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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 )
Expand All @@ -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 ) );
}
}

Expand All @@ -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();
}
}

0 comments on commit e2b88f1

Please sign in to comment.