Skip to content

Commit fbf4231

Browse files
ilgrossoilgrosso
committed
Checking that AccessToken owner matches JWT subject for admin
1 parent 967ef73 commit fbf4231

3 files changed

Lines changed: 164 additions & 42 deletions

File tree

‎core/spring/src/main/java/org/apache/syncope/core/spring/security/AuthDataAccessor.java‎

Lines changed: 45 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import org.apache.syncope.core.persistence.api.dao.UserDAO;
5050
import org.apache.syncope.core.persistence.api.dao.search.AttrCond;
5151
import org.apache.syncope.core.persistence.api.dao.search.SearchCond;
52+
import org.apache.syncope.core.persistence.api.entity.AccessToken;
5253
import org.apache.syncope.core.persistence.api.entity.Delegation;
5354
import org.apache.syncope.core.persistence.api.entity.DynRealm;
5455
import org.apache.syncope.core.persistence.api.entity.ExternalResource;
@@ -164,7 +165,7 @@ public JWTSSOProvider getJWTSSOProvider(final String issuer) {
164165

165166
return jwtSSOProviders.stream().filter(provider -> issuer.equals(provider.getIssuer())).findFirst().
166167
orElseThrow(() -> new AuthenticationCredentialsNotFoundException(
167-
"Could not find any registered JWTSSOProvider for issuer " + issuer));
168+
"Could not find any registered JWTSSOProvider for issuer " + issuer));
168169
}
169170

170171
protected String getDelegationKey(final SyncopeAuthenticationDetails details, final String delegatedKey) {
@@ -184,7 +185,7 @@ protected String getDelegationKey(final SyncopeAuthenticationDetails details, fi
184185

185186
return delegationDAO.findValidFor(delegatingKey, delegatedKey, OffsetDateTime.now()).
186187
orElseThrow(() -> new SessionAuthenticationException(
187-
"Delegation by " + delegatingKey + " was requested but none found"));
188+
"Delegation by " + delegatingKey + " was requested but none found"));
188189
}
189190

190191
/**
@@ -277,10 +278,10 @@ protected boolean usernamePasswordAuthentication(final User user, final String p
277278
try {
278279
Provision provision = resource.getProvisionByAnyType(AnyTypeKind.USER.name()).
279280
orElseThrow(() -> new AccountNotFoundException(
280-
"Unable to locate provision for user type " + AnyTypeKind.USER.name()));
281+
"Unable to locate provision for user type " + AnyTypeKind.USER.name()));
281282
connObjectKey = mappingManager.getConnObjectKeyValue(user, resource, provision).
282283
orElseThrow(() -> new AccountNotFoundException(
283-
"Unable to locate conn object key value for " + AnyTypeKind.USER.name()));
284+
"Unable to locate conn object key value for " + AnyTypeKind.USER.name()));
284285
Uid uid = connectorManager.getConnector(resource).authenticate(connObjectKey, password, null);
285286
if (uid != null) {
286287
authenticated = true;
@@ -356,31 +357,31 @@ protected Set<SyncopeGrantedAuthority> getUserAuthorities(final User user) {
356357
userDAO.findAllRoles(user).stream().
357358
filter(role -> !RoleDAO.GROUP_OWNER_ROLE.equals(role.getKey())).
358359
forEach(role -> role.getEntitlements().forEach(entitlement -> {
359-
Set<String> realms = Optional.ofNullable(entForRealms.get(entitlement)).orElseGet(() -> {
360-
Set<String> r = new HashSet<>();
361-
entForRealms.put(entitlement, r);
362-
return r;
363-
});
364-
365-
realms.addAll(role.getRealms().stream().map(Realm::getFullPath).collect(Collectors.toSet()));
366-
if (!entitlement.endsWith("_CREATE") && !entitlement.endsWith("_DELETE")) {
367-
realms.addAll(role.getDynRealms().stream().map(DynRealm::getKey).toList());
368-
}
369-
}));
370-
371-
// Give group entitlements for owned groups
372-
groupDAO.findOwnedByUser(user.getKey()).
373-
forEach(g -> roleDAO.findById(RoleDAO.GROUP_OWNER_ROLE).ifPresentOrElse(
374-
groupOwnerRole -> groupOwnerRole.getEntitlements().forEach(entitlement -> {
375360
Set<String> realms = Optional.ofNullable(entForRealms.get(entitlement)).orElseGet(() -> {
376-
HashSet<String> r = new HashSet<>();
361+
Set<String> r = new HashSet<>();
377362
entForRealms.put(entitlement, r);
378363
return r;
379364
});
380365

381-
realms.add(new RealmUtils.GroupOwnerRealm(g.getRealm().getFullPath(), g.getKey()).output());
382-
}),
383-
() -> LOG.warn("Role {} was not found", RoleDAO.GROUP_OWNER_ROLE)));
366+
realms.addAll(role.getRealms().stream().map(Realm::getFullPath).collect(Collectors.toSet()));
367+
if (!entitlement.endsWith("_CREATE") && !entitlement.endsWith("_DELETE")) {
368+
realms.addAll(role.getDynRealms().stream().map(DynRealm::getKey).toList());
369+
}
370+
}));
371+
372+
// Give group entitlements for owned groups
373+
groupDAO.findOwnedByUser(user.getKey()).
374+
forEach(g -> roleDAO.findById(RoleDAO.GROUP_OWNER_ROLE).ifPresentOrElse(
375+
groupOwnerRole -> groupOwnerRole.getEntitlements().forEach(entitlement -> {
376+
Set<String> realms = Optional.ofNullable(entForRealms.get(entitlement)).orElseGet(() -> {
377+
HashSet<String> r = new HashSet<>();
378+
entForRealms.put(entitlement, r);
379+
return r;
380+
});
381+
382+
realms.add(new RealmUtils.GroupOwnerRealm(g.getRealm().getFullPath(), g.getKey()).output());
383+
}),
384+
() -> LOG.warn("Role {} was not found", RoleDAO.GROUP_OWNER_ROLE)));
384385

385386
return buildAuthorities(entForRealms);
386387
}
@@ -390,17 +391,17 @@ protected Set<SyncopeGrantedAuthority> getDelegatedAuthorities(final Delegation
390391

391392
delegation.getRoles().stream().filter(role -> !RoleDAO.GROUP_OWNER_ROLE.equals(role.getKey())).
392393
forEach(role -> role.getEntitlements().forEach(entitlement -> {
393-
Set<String> realms = Optional.ofNullable(entForRealms.get(entitlement)).orElseGet(() -> {
394-
HashSet<String> r = new HashSet<>();
395-
entForRealms.put(entitlement, r);
396-
return r;
397-
});
398-
399-
realms.addAll(role.getRealms().stream().map(Realm::getFullPath).collect(Collectors.toSet()));
400-
if (!entitlement.endsWith("_CREATE") && !entitlement.endsWith("_DELETE")) {
401-
realms.addAll(role.getDynRealms().stream().map(DynRealm::getKey).toList());
402-
}
403-
}));
394+
Set<String> realms = Optional.ofNullable(entForRealms.get(entitlement)).orElseGet(() -> {
395+
HashSet<String> r = new HashSet<>();
396+
entForRealms.put(entitlement, r);
397+
return r;
398+
});
399+
400+
realms.addAll(role.getRealms().stream().map(Realm::getFullPath).collect(Collectors.toSet()));
401+
if (!entitlement.endsWith("_CREATE") && !entitlement.endsWith("_DELETE")) {
402+
realms.addAll(role.getDynRealms().stream().map(DynRealm::getKey).toList());
403+
}
404+
}));
404405

405406
return buildAuthorities(entForRealms);
406407
}
@@ -416,15 +417,15 @@ public Set<SyncopeGrantedAuthority> getAuthorities(final String username, final
416417
} else if (delegationKey != null) {
417418
Delegation delegation = delegationDAO.findById(delegationKey).
418419
orElseThrow(() -> new UsernameNotFoundException(
419-
"Could not find delegation " + delegationKey));
420+
"Could not find delegation " + delegationKey));
420421

421422
authorities = delegation.getRoles().isEmpty()
422423
? getUserAuthorities(delegation.getDelegating())
423424
: getDelegatedAuthorities(delegation);
424425
} else {
425426
User user = userDAO.findByUsername(username).
426427
orElseThrow(() -> new UsernameNotFoundException(
427-
"Could not find any user with username " + username));
428+
"Could not find any user with username " + username));
428429

429430
authorities = getUserAuthorities(user);
430431
}
@@ -438,18 +439,21 @@ public JWTAuthResult authenticate(final JWTAuthentication authentication) {
438439
Set<SyncopeGrantedAuthority> authorities;
439440

440441
if (securityProperties.getAdminUser().equals(authentication.getClaims().getSubject())) {
441-
accessTokenDAO.findById(authentication.getClaims().getJWTID()).
442+
AccessToken accessToken = accessTokenDAO.findById(authentication.getClaims().getJWTID()).
442443
orElseThrow(() -> new AuthenticationCredentialsNotFoundException(
443-
"Could not find an Access Token for JWT " + authentication.getClaims().getJWTID()));
444+
"Could not find an Access Token for JWT " + authentication.getClaims().getJWTID()));
445+
if (!securityProperties.getAdminUser().equals(accessToken.getOwner())) {
446+
throw new AuthenticationCredentialsNotFoundException("Access Token owner does not match JWT subject");
447+
}
444448

445449
username = securityProperties.getAdminUser();
446450
authorities = getAdminAuthorities();
447451
} else {
448452
JWTSSOProvider jwtSSOProvider = getJWTSSOProvider(authentication.getClaims().getIssuer());
449453
JWTSSOProvider.ResolvedClaims resolved = jwtSSOProvider.resolve(authentication.getClaims()).
450454
orElseThrow(() -> new AuthenticationCredentialsNotFoundException(
451-
"Could not find User " + authentication.getClaims().getSubject()
452-
+ " for JWT " + authentication.getClaims().getJWTID()));
455+
"Could not find User " + authentication.getClaims().getSubject()
456+
+ " for JWT " + authentication.getClaims().getJWTID()));
453457

454458
User user = resolved.user();
455459
String delegationKey = getDelegationKey(authentication.getDetails(), user.getKey());

‎core/spring/src/main/java/org/apache/syncope/core/spring/security/SyncopeJWTSSOProvider.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ public boolean verify(
100100
public Optional<ResolvedClaims> resolve(final JWTClaimsSet jwtClaims) {
101101
AccessToken accessToken = accessTokenDAO.findById(jwtClaims.getJWTID()).
102102
orElseThrow(() -> new AuthenticationCredentialsNotFoundException(
103-
"Could not find an Access Token for JWT " + jwtClaims.getJWTID()));
103+
"Could not find an Access Token for JWT " + jwtClaims.getJWTID()));
104104

105105
Set<SyncopeGrantedAuthority> authorities = new HashSet<>();
106106
if (accessToken.getAuthorities() != null) {

‎core/spring/src/test/java/org/apache/syncope/core/spring/security/AuthDataAccessorTest.java‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.syncope.core.spring.security;
20+
21+
import static org.junit.jupiter.api.Assertions.assertEquals;
22+
import static org.junit.jupiter.api.Assertions.assertThrows;
23+
import static org.junit.jupiter.api.Assertions.assertTrue;
24+
import static org.mockito.Mockito.doReturn;
25+
import static org.mockito.Mockito.mock;
26+
import static org.mockito.Mockito.when;
27+
28+
import com.nimbusds.jose.JWSAlgorithm;
29+
import com.nimbusds.jose.JWSHeader;
30+
import com.nimbusds.jwt.JWTClaimsSet;
31+
import com.nimbusds.jwt.SignedJWT;
32+
import java.lang.reflect.Constructor;
33+
import java.time.OffsetDateTime;
34+
import java.util.Arrays;
35+
import java.util.Date;
36+
import java.util.List;
37+
import java.util.Optional;
38+
import org.apache.syncope.common.lib.SyncopeConstants;
39+
import org.apache.syncope.common.lib.types.EntitlementsHolder;
40+
import org.apache.syncope.common.lib.types.IdRepoEntitlement;
41+
import org.apache.syncope.core.persistence.api.dao.AccessTokenDAO;
42+
import org.apache.syncope.core.persistence.api.entity.AccessToken;
43+
import org.apache.syncope.core.spring.security.jws.AccessTokenJWSSigner;
44+
import org.apache.syncope.core.spring.security.jws.AccessTokenJWSVerifier;
45+
import org.junit.jupiter.api.BeforeEach;
46+
import org.junit.jupiter.api.Test;
47+
import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
48+
49+
class AuthDataAccessorTest {
50+
51+
private static final String LOW_PRIVILEGE_JTI = "low-privilege-users-valid-token-id";
52+
53+
private static final String LOW_PRIVILEGE_USER = "low-priv-user";
54+
55+
private SecurityProperties securityProperties;
56+
57+
private AccessTokenDAO accessTokenDAO;
58+
59+
private AuthDataAccessor authDataAccessor;
60+
61+
@BeforeEach
62+
void setUp() throws Exception {
63+
securityProperties = new SecurityProperties();
64+
securityProperties.setJwtIssuer("ApacheSyncope");
65+
securityProperties.setJwsAlgorithm("HS512");
66+
securityProperties.setJwsKey(SecureRandomUtils.generateRandomLetters(64));
67+
securityProperties.setAdminUser("admin");
68+
69+
EntitlementsHolder.getInstance().addAll(IdRepoEntitlement.values());
70+
71+
AccessToken lowPrivilegeToken = mock(AccessToken.class);
72+
when(lowPrivilegeToken.getOwner()).thenReturn(LOW_PRIVILEGE_USER);
73+
accessTokenDAO = mock(AccessTokenDAO.class);
74+
doReturn(Optional.of(lowPrivilegeToken)).when(accessTokenDAO).findById(LOW_PRIVILEGE_JTI);
75+
76+
Constructor<?> constructor = Arrays.stream(AuthDataAccessor.class.getConstructors()).
77+
findFirst().orElseThrow();
78+
Object[] arguments = Arrays.stream(constructor.getParameterTypes()).map(type -> {
79+
if (type == SecurityProperties.class) {
80+
return securityProperties;
81+
}
82+
if (type == AccessTokenDAO.class) {
83+
return accessTokenDAO;
84+
}
85+
if (type == List.class) {
86+
return List.of();
87+
}
88+
return mock(type);
89+
}).toArray();
90+
91+
authDataAccessor = (AuthDataAccessor) constructor.newInstance(arguments);
92+
}
93+
94+
@Test
95+
void lowPrivilegeTokenIdCanBeReboundToAdminWithDefaultSigningKey() throws Exception {
96+
JWTClaimsSet claims = new JWTClaimsSet.Builder().
97+
issuer(securityProperties.getJwtIssuer()).
98+
subject(securityProperties.getAdminUser()).
99+
jwtID(LOW_PRIVILEGE_JTI).
100+
issueTime(new Date()).
101+
notBeforeTime(new Date()).
102+
expirationTime(Date.from(OffsetDateTime.now().plusMinutes(10).toInstant())).
103+
build();
104+
SignedJWT forged = new SignedJWT(new JWSHeader(JWSAlgorithm.HS512), claims);
105+
forged.sign(new AccessTokenJWSSigner(JWSAlgorithm.HS512, securityProperties.getJwsKey()));
106+
107+
assertTrue(forged.verify(new AccessTokenJWSVerifier(JWSAlgorithm.HS512, securityProperties.getJwsKey())));
108+
109+
JWTAuthentication authentication = new JWTAuthentication(
110+
forged.getJWTClaimsSet(),
111+
new SyncopeAuthenticationDetails(SyncopeConstants.MASTER_DOMAIN, null));
112+
113+
AuthenticationCredentialsNotFoundException ex = assertThrows(
114+
AuthenticationCredentialsNotFoundException.class,
115+
() -> authDataAccessor.authenticate(authentication));
116+
assertEquals("Access Token owner does not match JWT subject", ex.getMessage());
117+
}
118+
}

0 commit comments

Comments
 (0)