From 74bc271ec2beb6c7b30bfe7337b845ddc2ea392a Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Tue, 21 Jun 2022 16:35:11 -0600 Subject: [PATCH] Use SecurityContextHolderStrategy for ACL Issue gh-11060 --- .../domain/AclAuthorizationStrategyImpl.java | 24 +++++++++++++++---- .../acls/jdbc/JdbcMutableAclService.java | 17 ++++++++++++- .../AclAuthorizationStrategyImplTests.java | 24 +++++++++++++++++-- .../acls/jdbc/JdbcMutableAclServiceTests.java | 18 ++++++++++++++ 4 files changed, 76 insertions(+), 7 deletions(-) diff --git a/acl/src/main/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImpl.java b/acl/src/main/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImpl.java index 4753f973f4..c39f2a1d2c 100644 --- a/acl/src/main/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImpl.java +++ b/acl/src/main/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImpl.java @@ -27,7 +27,9 @@ import org.springframework.security.acls.model.SidRetrievalStrategy; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; 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.SecurityContextHolderStrategy; import org.springframework.util.Assert; /** @@ -46,6 +48,9 @@ import org.springframework.util.Assert; */ public class AclAuthorizationStrategyImpl implements AclAuthorizationStrategy { + private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder + .getContextHolderStrategy(); + private final GrantedAuthority gaGeneralChanges; private final GrantedAuthority gaModifyAuditing; @@ -81,12 +86,12 @@ public class AclAuthorizationStrategyImpl implements AclAuthorizationStrategy { @Override public void securityCheck(Acl acl, int changeType) { - if ((SecurityContextHolder.getContext() == null) - || (SecurityContextHolder.getContext().getAuthentication() == null) - || !SecurityContextHolder.getContext().getAuthentication().isAuthenticated()) { + SecurityContext context = this.securityContextHolderStrategy.getContext(); + if ((context == null) || (context.getAuthentication() == null) + || !context.getAuthentication().isAuthenticated()) { 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 Sid currentUser = createCurrentUser(authentication); if (currentUser.equals(acl.getOwner()) @@ -146,4 +151,15 @@ public class AclAuthorizationStrategyImpl implements AclAuthorizationStrategy { 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; + } + } diff --git a/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcMutableAclService.java b/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcMutableAclService.java index f1c7a7a174..ff27aea99c 100644 --- a/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcMutableAclService.java +++ b/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcMutableAclService.java @@ -40,6 +40,7 @@ import org.springframework.security.acls.model.ObjectIdentity; import org.springframework.security.acls.model.Sid; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.core.context.SecurityContextHolderStrategy; import org.springframework.transaction.support.TransactionSynchronizationManager; 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 SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder + .getContextHolderStrategy(); + private boolean foreignKeysInDatabase = true; 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 // (can be changed later on) - Authentication auth = SecurityContextHolder.getContext().getAuthentication(); + Authentication auth = this.securityContextHolderStrategy.getContext().getAuthentication(); PrincipalSid sid = new PrincipalSid(auth); // 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; + } + } diff --git a/acl/src/test/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImplTests.java b/acl/src/test/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImplTests.java index 5d7201251f..9a61e6d250 100644 --- a/acl/src/test/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImplTests.java +++ b/acl/src/test/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImplTests.java @@ -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"); * 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.core.GrantedAuthority; 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.SecurityContextHolderStrategy; +import org.springframework.security.core.context.SecurityContextImpl; import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.verify; /** * @author Rob Winch @@ -40,9 +44,14 @@ import static org.mockito.BDDMockito.given; @ExtendWith(MockitoExtension.class) public class AclAuthorizationStrategyImplTests { + SecurityContext context; + @Mock Acl acl; + @Mock + SecurityContextHolderStrategy securityContextHolderStrategy; + GrantedAuthority authority; AclAuthorizationStrategyImpl strategy; @@ -53,7 +62,8 @@ public class AclAuthorizationStrategyImplTests { TestingAuthenticationToken authentication = new TestingAuthenticationToken("foo", "bar", Arrays.asList(this.authority)); authentication.setAuthenticated(true); - SecurityContextHolder.getContext().setAuthentication(authentication); + this.context = new SecurityContextImpl(authentication); + SecurityContextHolder.setContext(this.context); } @AfterEach @@ -76,6 +86,16 @@ public class AclAuthorizationStrategyImplTests { 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") class CustomAuthority implements GrantedAuthority { diff --git a/acl/src/test/java/org/springframework/security/acls/jdbc/JdbcMutableAclServiceTests.java b/acl/src/test/java/org/springframework/security/acls/jdbc/JdbcMutableAclServiceTests.java index 9db8002a8c..3480ef84c1 100644 --- a/acl/src/test/java/org/springframework/security/acls/jdbc/JdbcMutableAclServiceTests.java +++ b/acl/src/test/java/org/springframework/security/acls/jdbc/JdbcMutableAclServiceTests.java @@ -48,7 +48,10 @@ import org.springframework.security.acls.model.Sid; import org.springframework.security.acls.sid.CustomSid; import org.springframework.security.authentication.TestingAuthenticationToken; 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.SecurityContextHolderStrategy; +import org.springframework.security.core.context.SecurityContextImpl; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; 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.assertThatIllegalArgumentException; import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; /** * 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(); } + @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 */