Skip to content

Commit 018244c

Browse files
LEDfanLEDfan
committed
Fix #34710: align case-sensitivity of username
1 parent e0deca9 commit 018244c

10 files changed

Lines changed: 85 additions & 18 deletions

File tree

‎src/main/java/eu/openanalytics/containerproxy/MemoryStoreConfiguration.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import eu.openanalytics.containerproxy.model.store.IProxyStore;
2525
import eu.openanalytics.containerproxy.model.store.memory.MemoryHeartbeatStore;
2626
import eu.openanalytics.containerproxy.model.store.memory.MemoryProxyStore;
27+
import eu.openanalytics.containerproxy.service.AccessControlEvaluationService;
2728
import eu.openanalytics.containerproxy.service.leader.memory.MemoryLeaderService;
2829
import eu.openanalytics.containerproxy.service.portallocator.memory.MemoryPortAllocator;
2930
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
@@ -37,8 +38,8 @@
3738
public class MemoryStoreConfiguration {
3839

3940
@Bean
40-
public IProxyStore proxyStore() {
41-
return new MemoryProxyStore();
41+
public IProxyStore proxyStore(AccessControlEvaluationService accessControlEvaluationService) {
42+
return new MemoryProxyStore(accessControlEvaluationService);
4243
}
4344

4445
@Bean

‎src/main/java/eu/openanalytics/containerproxy/RedisStoreConfiguration.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import eu.openanalytics.containerproxy.model.store.IProxyStore;
3232
import eu.openanalytics.containerproxy.model.store.redis.RedisHeartbeatStore;
3333
import eu.openanalytics.containerproxy.model.store.redis.RedisProxyStore;
34+
import eu.openanalytics.containerproxy.service.AccessControlEvaluationService;
3435
import eu.openanalytics.containerproxy.service.IdentifierService;
3536
import eu.openanalytics.containerproxy.service.RedisEventBridge;
3637
import eu.openanalytics.containerproxy.service.leader.GlobalEventLoopService;
@@ -79,8 +80,8 @@ public class RedisStoreConfiguration {
7980
// Store beans
8081

8182
@Bean
82-
public IProxyStore proxyStore() {
83-
return new RedisProxyStore();
83+
public IProxyStore proxyStore(AccessControlEvaluationService accessControlEvaluationService) {
84+
return new RedisProxyStore(accessControlEvaluationService);
8485
}
8586

8687
@Bean

‎src/main/java/eu/openanalytics/containerproxy/model/store/memory/MemoryProxyStore.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.google.common.collect.Multimaps;
2626
import eu.openanalytics.containerproxy.model.runtime.Proxy;
2727
import eu.openanalytics.containerproxy.model.store.IProxyStore;
28+
import eu.openanalytics.containerproxy.service.AccessControlEvaluationService;
2829

2930
import java.util.ArrayList;
3031
import java.util.Collection;
@@ -36,6 +37,11 @@ public class MemoryProxyStore implements IProxyStore {
3637

3738
private final ConcurrentHashMap<String, Proxy> activeProxies = new ConcurrentHashMap<>();
3839
private final ListMultimap<String, String> userProxies = Multimaps.synchronizedListMultimap(ArrayListMultimap.create());
40+
private final AccessControlEvaluationService accessControlEvaluationService;
41+
42+
public MemoryProxyStore(AccessControlEvaluationService accessControlEvaluationService) {
43+
this.accessControlEvaluationService = accessControlEvaluationService;
44+
}
3945

4046
@Override
4147
public Collection<Proxy> getAllProxies() {
@@ -70,7 +76,7 @@ public List<Proxy> getUserProxies(String userId) {
7076
List<String> ids = userProxies.get(userId);
7177
for (String proxyId : ids) {
7278
Proxy proxy = activeProxies.get(proxyId);
73-
if (proxy != null && proxy.getUserId().equalsIgnoreCase(userId)) {
79+
if (proxy != null && accessControlEvaluationService.usernameEquals(proxy.getUserId(), userId)) {
7480
result.add(proxy);
7581
}
7682
}

‎src/main/java/eu/openanalytics/containerproxy/model/store/redis/RedisProxyStore.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import eu.openanalytics.containerproxy.event.ProxyStopEvent;
2424
import eu.openanalytics.containerproxy.model.runtime.Proxy;
2525
import eu.openanalytics.containerproxy.model.store.IProxyStore;
26+
import eu.openanalytics.containerproxy.service.AccessControlEvaluationService;
2627
import eu.openanalytics.containerproxy.service.IdentifierService;
2728
import eu.openanalytics.containerproxy.util.ProxyHashMap;
2829
import eu.openanalytics.containerproxy.util.ProxyMappingManager;
@@ -43,6 +44,7 @@
4344
public class RedisProxyStore implements IProxyStore {
4445

4546
private final Logger logger = LogManager.getLogger(RedisProxyStore.class);
47+
private final AccessControlEvaluationService accessControlEvaluationService;
4648
@Inject
4749
private RedisTemplate<String, Proxy> redisTemplate;
4850
@Inject
@@ -58,6 +60,10 @@ public class RedisProxyStore implements IProxyStore {
5860
private final ConcurrentHashMap<String, Proxy> cache = ProxyHashMap.create();
5961
private String userProxyRedisKey;
6062

63+
public RedisProxyStore(AccessControlEvaluationService accessControlEvaluationService) {
64+
this.accessControlEvaluationService = accessControlEvaluationService;
65+
}
66+
6167
@PostConstruct
6268
public void init() {
6369
redisKey = "shinyproxy_" + identifierService.realmId + "__active_proxies";
@@ -114,7 +120,7 @@ public List<Proxy> getUserProxies(String userId) {
114120
}
115121
List<Proxy> proxies = ops.multiGet(redisKey, ids);
116122
for (Proxy proxy : proxies) {
117-
if (proxy != null && proxy.getUserId().equalsIgnoreCase(userId)) {
123+
if (proxy != null && accessControlEvaluationService.usernameEquals(proxy.getUserId(), userId)) {
118124
result.add(proxy);
119125
}
120126
}

‎src/main/java/eu/openanalytics/containerproxy/service/AccessControlEvaluationService.java‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,22 +26,27 @@
2626
import eu.openanalytics.containerproxy.spec.expression.SpecExpressionContext;
2727
import eu.openanalytics.containerproxy.spec.expression.SpecExpressionResolver;
2828
import org.springframework.context.annotation.Lazy;
29+
import org.springframework.core.env.Environment;
2930
import org.springframework.security.authentication.AnonymousAuthenticationToken;
3031
import org.springframework.security.core.Authentication;
3132
import org.springframework.stereotype.Service;
3233

3334
@Service
3435
public class AccessControlEvaluationService {
3536

37+
public static final String PROP_USERNAME_CASE_SENSITIVE = "proxy.username-case-sensitive";
38+
3639
private final IAuthenticationBackend authBackend;
3740
private final UserService userService;
3841

3942
private final SpecExpressionResolver specExpressionResolver;
43+
private final Boolean usernameCaseSensitive;
4044

41-
public AccessControlEvaluationService(@Lazy IAuthenticationBackend authBackend, UserService userService, SpecExpressionResolver specExpressionResolver) {
45+
public AccessControlEvaluationService(@Lazy IAuthenticationBackend authBackend, UserService userService, SpecExpressionResolver specExpressionResolver, Environment environment) {
4246
this.authBackend = authBackend;
4347
this.userService = userService;
4448
this.specExpressionResolver = specExpressionResolver;
49+
usernameCaseSensitive = environment.getProperty(PROP_USERNAME_CASE_SENSITIVE, Boolean.class, true);
4550
}
4651

4752
public boolean checkAccess(Authentication auth, ProxySpec spec, AccessControl accessControl, Object... objects) {
@@ -96,7 +101,7 @@ public boolean allowedByUsers(Authentication auth, AccessControl accessControl)
96101
return false;
97102
}
98103
for (String user : accessControl.getUsers()) {
99-
if (auth.getName().equals(user)) {
104+
if (usernameEquals(auth.getName(), user)) {
100105
return true;
101106
}
102107
}
@@ -119,4 +124,11 @@ public boolean allowedByExpression(Authentication auth, ProxySpec spec, AccessCo
119124
return specExpressionResolver.evaluateToBoolean(accessControl.getExpression(), contextBuilder.build());
120125
}
121126

127+
public boolean usernameEquals(String authenticatedUser, String other) {
128+
if (usernameCaseSensitive) {
129+
return authenticatedUser.equals(other);
130+
}
131+
return authenticatedUser.equalsIgnoreCase(other);
132+
}
133+
122134
}

‎src/main/java/eu/openanalytics/containerproxy/service/ProxyIdIndex.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,11 @@ public abstract class ProxyIdIndex<T> {
3636
private final LoadingCache<T, String> cache;
3737
private final IProxyStore proxyStore;
3838
private final Logger logger = LoggerFactory.getLogger(getClass());
39+
private final AccessControlEvaluationService accessControlEvaluationService;
3940

40-
public ProxyIdIndex(IProxyStore proxyStore, Filter<T> filter) {
41+
public ProxyIdIndex(IProxyStore proxyStore, AccessControlEvaluationService accessControlEvaluationService, Filter<T> filter) {
4142
this.proxyStore = proxyStore;
43+
this.accessControlEvaluationService = accessControlEvaluationService;
4244
cache = Caffeine.newBuilder()
4345
.scheduler(Scheduler.systemScheduler())
4446
.expireAfterAccess(10, TimeUnit.MINUTES)
@@ -93,7 +95,7 @@ private boolean isInvalidProxy(Proxy proxy, String proxyId, String userId) {
9395
logger.warn("Invalid proxy state, proxyId: {}, does not match expected: {}", proxy.getId(), proxyId);
9496
return true;
9597
}
96-
if (!proxy.getUserId().equals(userId)) {
98+
if (!accessControlEvaluationService.usernameEquals(proxy.getUserId(), userId)) {
9799
logger.warn("Invalid proxy state for proxyId: {}, userId: {} does not match expected: {}", proxyId, proxy.getUserId(), userId);
98100
return true;
99101
}

‎src/main/java/eu/openanalytics/containerproxy/service/UserAndTargetIdProxyIndex.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@
2929
@Component
3030
public class UserAndTargetIdProxyIndex extends ProxyIdIndex<UserAndTargetIdProxyIndex.UserAndTargetIdKey> {
3131

32-
public UserAndTargetIdProxyIndex(IProxyStore proxyStore) {
33-
super(proxyStore, (key, proxy) -> Objects.equals(proxy.getTargetId(), key.targetId) && Objects.equals(proxy.getUserId(), key.userId));
32+
public UserAndTargetIdProxyIndex(IProxyStore proxyStore, AccessControlEvaluationService accessControlEvaluationService) {
33+
super(proxyStore, accessControlEvaluationService, (key, proxy) -> Objects.equals(proxy.getTargetId(), key.targetId) && Objects.equals(proxy.getUserId(), key.userId));
3434
// use Objects.equals because some proxies might not yet be initialized
3535
}
3636

‎src/main/java/eu/openanalytics/containerproxy/service/UserService.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ public class UserService {
8181
@Inject
8282
@Lazy
8383
private ProxyAccessControlService accessControlService;
84+
@Inject
85+
@Lazy
86+
private AccessControlEvaluationService accessControlEvaluationService;
8487

8588
public UserService() {
8689
// cache isAdmin status results for (at least) 60 minutes, since this never changes during the lifetime of a session
@@ -174,7 +177,7 @@ public boolean isAdmin(Authentication auth) {
174177
}
175178

176179
for (String adminUser : getAdminUsers()) {
177-
if (userName.equalsIgnoreCase(adminUser)) {
180+
if (accessControlEvaluationService.usernameEquals(userName, adminUser)) {
178181
return true;
179182
}
180183
}
@@ -195,8 +198,10 @@ public boolean isOwner(Proxy proxy) {
195198
}
196199

197200
public boolean isOwner(Authentication auth, Proxy proxy) {
198-
if (auth == null || proxy == null) return false;
199-
return proxy.getUserId().equalsIgnoreCase(auth.getName());
201+
if (auth == null || proxy == null) {
202+
return false;
203+
}
204+
return accessControlEvaluationService.usernameEquals(proxy.getUserId(), auth.getName());
200205
}
201206

202207
public boolean isMember(Authentication auth, String groupName) {

‎src/test/java/eu/openanalytics/containerproxy/test/auth/AccessControlServiceTest.java‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,15 @@
3232
import org.junit.jupiter.api.Assertions;
3333
import org.junit.jupiter.api.Test;
3434
import org.springframework.context.support.GenericApplicationContext;
35+
import org.springframework.core.env.Environment;
3536
import org.springframework.security.authentication.AnonymousAuthenticationToken;
3637
import org.springframework.security.core.Authentication;
3738
import org.springframework.security.core.authority.SimpleGrantedAuthority;
3839

3940
import java.util.Collection;
4041
import java.util.Collections;
4142

43+
import static eu.openanalytics.containerproxy.service.AccessControlEvaluationService.PROP_USERNAME_CASE_SENSITIVE;
4244
import static org.mockito.Mockito.mock;
4345
import static org.mockito.Mockito.when;
4446

@@ -48,15 +50,25 @@ public class AccessControlServiceTest {
4850
private final UserService userService;
4951
private final IProxySpecProvider specProvider;
5052
private final ProxyAccessControlService accessControlService;
53+
private final ProxyAccessControlService accessControlServiceCaseInsensitive;
5154
private final ProxyService proxyService;
55+
private final Environment environment;
5256

5357
public AccessControlServiceTest() {
5458
authBackend = mock(IAuthenticationBackend.class);
5559
userService = mock(UserService.class);
5660
specProvider = mock(IProxySpecProvider.class);
5761
proxyService = mock(ProxyService.class);
62+
environment = mock(Environment.class);
5863
SpecExpressionResolver specExpressionResolver = new SpecExpressionResolver(new GenericApplicationContext());
59-
accessControlService = new ProxyAccessControlService(proxyService, specProvider, new AccessControlEvaluationService(authBackend, userService, specExpressionResolver));
64+
65+
// case-sensitive access controller
66+
when(environment.getProperty(PROP_USERNAME_CASE_SENSITIVE, Boolean.class, true)).thenReturn(true);
67+
accessControlService = new ProxyAccessControlService(proxyService, specProvider, new AccessControlEvaluationService(authBackend, userService, specExpressionResolver, environment));
68+
69+
// case-insensitive access controller
70+
when(environment.getProperty(PROP_USERNAME_CASE_SENSITIVE, Boolean.class, true)).thenReturn(false);
71+
accessControlServiceCaseInsensitive = new ProxyAccessControlService(proxyService, specProvider, new AccessControlEvaluationService(authBackend, userService, specExpressionResolver, environment));
6072
}
6173

6274
@Test
@@ -149,7 +161,7 @@ public void hasGroupAccessTest() {
149161
}
150162

151163
@Test
152-
public void hasUserAccessTest() {
164+
public void hasUserAccessTestCaseSensitive() {
153165
when(authBackend.hasAuthorization()).thenReturn(true);
154166
AccessControl proxyAccessControl = new AccessControl();
155167
proxyAccessControl.setUsers(new String[]{"myUser1", "myUser2"});
@@ -164,6 +176,24 @@ public void hasUserAccessTest() {
164176
when(auth2.getName()).thenReturn("myUser1");
165177
when(userService.isMember(auth2, "myGroup1")).thenReturn(true);
166178
Assertions.assertTrue(accessControlService.canAccess(auth2, createProxySpec(proxyAccessControl)));
179+
180+
// test should be case-sensitive
181+
Authentication auth3 = mock(Authentication.class);
182+
when(auth3.getName()).thenReturn("myuser1");
183+
when(userService.isMember(auth3, "myGroup1")).thenReturn(true);
184+
Assertions.assertFalse(accessControlService.canAccess(auth3, createProxySpec(proxyAccessControl)));
185+
}
186+
187+
@Test
188+
public void hasUserAccessTestCaseInSensitive() {
189+
when(authBackend.hasAuthorization()).thenReturn(true);
190+
AccessControl proxyAccessControl = new AccessControl();
191+
proxyAccessControl.setUsers(new String[]{"myUser1", "myUser2"});
192+
193+
// user in lowercase should have access
194+
Authentication auth1 = mock(Authentication.class);
195+
when(auth1.getName()).thenReturn("myuser1");
196+
Assertions.assertFalse(accessControlService.canAccess(auth1, createProxySpec(proxyAccessControl)));
167197
}
168198

169199
@Test

‎src/test/java/eu/openanalytics/containerproxy/test/unit/TestParameterServiceAccessControl.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.junit.jupiter.api.extension.ExtendWith;
3838
import org.springframework.boot.test.context.SpringBootTest;
3939
import org.springframework.context.support.GenericApplicationContext;
40+
import org.springframework.core.env.Environment;
4041
import org.springframework.security.core.Authentication;
4142
import org.springframework.security.core.authority.SimpleGrantedAuthority;
4243
import org.springframework.test.context.ActiveProfiles;
@@ -68,14 +69,17 @@ public class TestParameterServiceAccessControl {
6869
@Inject
6970
private ObjectMapper objectMapper;
7071

72+
@Inject
73+
private Environment environment;
74+
7175
private UserService userService;
7276

7377
@PostConstruct
7478
public void init() {
7579
authBackend = mock(IAuthenticationBackend.class);
7680
userService = mock(UserService.class);
7781
SpecExpressionResolver specExpressionResolver = new SpecExpressionResolver(new GenericApplicationContext());
78-
AccessControlEvaluationService accessControlEvaluationService = new AccessControlEvaluationService(authBackend, userService, specExpressionResolver);
82+
AccessControlEvaluationService accessControlEvaluationService = new AccessControlEvaluationService(authBackend, userService, specExpressionResolver, environment);
7983

8084
parametersService = new ParametersService(proxySpecProvider, accessControlEvaluationService, objectMapper);
8185
}

0 commit comments

Comments
 (0)