Use SecurityContextHolderStrategy for ACL

Issue gh-11060
This commit is contained in:
Josh Cummings 2022-06-21 16:35:11 -06:00
parent 94f51d0718
commit 52dc120269
No known key found for this signature in database
GPG Key ID: A306A51F43B8E5A5
4 changed files with 76 additions and 7 deletions

View File

@ -27,7 +27,9 @@ import org.springframework.security.acls.model.SidRetrievalStrategy;
import org.springframework.security.core.Authentication; import org.springframework.security.core.Authentication;
import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.core.authority.AuthorityUtils;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.core.context.SecurityContextHolderStrategy;
import org.springframework.util.Assert; import org.springframework.util.Assert;
/** /**
@ -46,6 +48,9 @@ import org.springframework.util.Assert;
*/ */
public class AclAuthorizationStrategyImpl implements AclAuthorizationStrategy { public class AclAuthorizationStrategyImpl implements AclAuthorizationStrategy {
private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder
.getContextHolderStrategy();
private final GrantedAuthority gaGeneralChanges; private final GrantedAuthority gaGeneralChanges;
private final GrantedAuthority gaModifyAuditing; private final GrantedAuthority gaModifyAuditing;
@ -81,12 +86,12 @@ public class AclAuthorizationStrategyImpl implements AclAuthorizationStrategy {
@Override @Override
public void securityCheck(Acl acl, int changeType) { public void securityCheck(Acl acl, int changeType) {
if ((SecurityContextHolder.getContext() == null) SecurityContext context = this.securityContextHolderStrategy.getContext();
|| (SecurityContextHolder.getContext().getAuthentication() == null) if ((context == null) || (context.getAuthentication() == null)
|| !SecurityContextHolder.getContext().getAuthentication().isAuthenticated()) { || !context.getAuthentication().isAuthenticated()) {
throw new AccessDeniedException("Authenticated principal required to operate with ACLs"); throw new AccessDeniedException("Authenticated principal required to operate with ACLs");
} }
Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); Authentication authentication = context.getAuthentication();
// Check if authorized by virtue of ACL ownership // Check if authorized by virtue of ACL ownership
Sid currentUser = createCurrentUser(authentication); Sid currentUser = createCurrentUser(authentication);
if (currentUser.equals(acl.getOwner()) if (currentUser.equals(acl.getOwner())
@ -146,4 +151,15 @@ public class AclAuthorizationStrategyImpl implements AclAuthorizationStrategy {
this.sidRetrievalStrategy = sidRetrievalStrategy; this.sidRetrievalStrategy = sidRetrievalStrategy;
} }
/**
* Sets the {@link SecurityContextHolderStrategy} to use. The default action is to use
* the {@link SecurityContextHolderStrategy} stored in {@link SecurityContextHolder}.
*
* @since 5.8
*/
public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy securityContextHolderStrategy) {
Assert.notNull(securityContextHolderStrategy, "securityContextHolderStrategy cannot be null");
this.securityContextHolderStrategy = securityContextHolderStrategy;
}
} }

View File

@ -40,6 +40,7 @@ import org.springframework.security.acls.model.ObjectIdentity;
import org.springframework.security.acls.model.Sid; import org.springframework.security.acls.model.Sid;
import org.springframework.security.core.Authentication; import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.core.context.SecurityContextHolderStrategy;
import org.springframework.transaction.support.TransactionSynchronizationManager; import org.springframework.transaction.support.TransactionSynchronizationManager;
import org.springframework.util.Assert; import org.springframework.util.Assert;
@ -64,6 +65,9 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
private static final String DEFAULT_INSERT_INTO_ACL_CLASS_WITH_ID = "insert into acl_class (class, class_id_type) values (?, ?)"; private static final String DEFAULT_INSERT_INTO_ACL_CLASS_WITH_ID = "insert into acl_class (class, class_id_type) values (?, ?)";
private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder
.getContextHolderStrategy();
private boolean foreignKeysInDatabase = true; private boolean foreignKeysInDatabase = true;
private final AclCache aclCache; private final AclCache aclCache;
@ -115,7 +119,7 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
// Need to retrieve the current principal, in order to know who "owns" this ACL // Need to retrieve the current principal, in order to know who "owns" this ACL
// (can be changed later on) // (can be changed later on)
Authentication auth = SecurityContextHolder.getContext().getAuthentication(); Authentication auth = this.securityContextHolderStrategy.getContext().getAuthentication();
PrincipalSid sid = new PrincipalSid(auth); PrincipalSid sid = new PrincipalSid(auth);
// Create the acl_object_identity row // Create the acl_object_identity row
@ -473,4 +477,15 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
} }
} }
/**
* Sets the {@link SecurityContextHolderStrategy} to use. The default action is to use
* the {@link SecurityContextHolderStrategy} stored in {@link SecurityContextHolder}.
*
* @since 5.8
*/
public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy securityContextHolderStrategy) {
Assert.notNull(securityContextHolderStrategy, "securityContextHolderStrategy cannot be null");
this.securityContextHolderStrategy = securityContextHolderStrategy;
}
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2016 the original author or authors. * Copyright 2002-2022 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -29,9 +29,13 @@ import org.springframework.security.acls.model.Acl;
import org.springframework.security.authentication.TestingAuthenticationToken; import org.springframework.security.authentication.TestingAuthenticationToken;
import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.core.context.SecurityContextHolderStrategy;
import org.springframework.security.core.context.SecurityContextImpl;
import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.verify;
/** /**
* @author Rob Winch * @author Rob Winch
@ -40,9 +44,14 @@ import static org.mockito.BDDMockito.given;
@ExtendWith(MockitoExtension.class) @ExtendWith(MockitoExtension.class)
public class AclAuthorizationStrategyImplTests { public class AclAuthorizationStrategyImplTests {
SecurityContext context;
@Mock @Mock
Acl acl; Acl acl;
@Mock
SecurityContextHolderStrategy securityContextHolderStrategy;
GrantedAuthority authority; GrantedAuthority authority;
AclAuthorizationStrategyImpl strategy; AclAuthorizationStrategyImpl strategy;
@ -53,7 +62,8 @@ public class AclAuthorizationStrategyImplTests {
TestingAuthenticationToken authentication = new TestingAuthenticationToken("foo", "bar", TestingAuthenticationToken authentication = new TestingAuthenticationToken("foo", "bar",
Arrays.asList(this.authority)); Arrays.asList(this.authority));
authentication.setAuthenticated(true); authentication.setAuthenticated(true);
SecurityContextHolder.getContext().setAuthentication(authentication); this.context = new SecurityContextImpl(authentication);
SecurityContextHolder.setContext(this.context);
} }
@AfterEach @AfterEach
@ -76,6 +86,16 @@ public class AclAuthorizationStrategyImplTests {
this.strategy.securityCheck(this.acl, AclAuthorizationStrategy.CHANGE_GENERAL); this.strategy.securityCheck(this.acl, AclAuthorizationStrategy.CHANGE_GENERAL);
} }
@Test
public void securityCheckWhenCustomSecurityContextHolderStrategyThenUses() {
given(this.securityContextHolderStrategy.getContext()).willReturn(this.context);
given(this.acl.getOwner()).willReturn(new GrantedAuthoritySid("ROLE_AUTH"));
this.strategy = new AclAuthorizationStrategyImpl(new SimpleGrantedAuthority("ROLE_SYSTEM_ADMIN"));
this.strategy.setSecurityContextHolderStrategy(this.securityContextHolderStrategy);
this.strategy.securityCheck(this.acl, AclAuthorizationStrategy.CHANGE_GENERAL);
verify(this.securityContextHolderStrategy).getContext();
}
@SuppressWarnings("serial") @SuppressWarnings("serial")
class CustomAuthority implements GrantedAuthority { class CustomAuthority implements GrantedAuthority {

View File

@ -48,7 +48,10 @@ import org.springframework.security.acls.model.Sid;
import org.springframework.security.acls.sid.CustomSid; import org.springframework.security.acls.sid.CustomSid;
import org.springframework.security.authentication.TestingAuthenticationToken; import org.springframework.security.authentication.TestingAuthenticationToken;
import org.springframework.security.core.Authentication; import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.core.context.SecurityContextHolderStrategy;
import org.springframework.security.core.context.SecurityContextImpl;
import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit.jupiter.SpringExtension; import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.test.context.transaction.AfterTransaction; import org.springframework.test.context.transaction.AfterTransaction;
@ -59,7 +62,9 @@ import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy; import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
/** /**
* Integration tests the ACL system using an in-memory database. * Integration tests the ACL system using an in-memory database.
@ -350,6 +355,19 @@ public class JdbcMutableAclServiceTests {
assertThat(this.jdbcMutableAclService.readAclById(new ObjectIdentityImpl(TARGET_CLASS, 101L))).isNotNull(); assertThat(this.jdbcMutableAclService.readAclById(new ObjectIdentityImpl(TARGET_CLASS, 101L))).isNotNull();
} }
@Test
@Transactional
public void createAclWhenCustomSecurityContextHolderStrategyThenUses() {
SecurityContextHolderStrategy securityContextHolderStrategy = mock(SecurityContextHolderStrategy.class);
SecurityContext context = new SecurityContextImpl(this.auth);
given(securityContextHolderStrategy.getContext()).willReturn(context);
JdbcMutableAclService service = new JdbcMutableAclService(this.dataSource, this.lookupStrategy, this.aclCache);
service.setSecurityContextHolderStrategy(securityContextHolderStrategy);
ObjectIdentity oid = new ObjectIdentityImpl(TARGET_CLASS, 101);
service.createAcl(oid);
verify(securityContextHolderStrategy).getContext();
}
/** /**
* SEC-655 * SEC-655
*/ */