Skip to content

Commit 86ad0ca

Browse files
committed
fix: Invalidate user role member sessions async without loading users [DHIS2-21854] (#24494)
(cherry picked from commit e4a9688)
1 parent 2a0ed9c commit 86ad0ca

17 files changed

Lines changed: 697 additions & 51 deletions

File tree

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright (c) 2004-2022, University of Oslo
3+
* All rights reserved.
4+
*
5+
* Redistribution and use in source and binary forms, with or without
6+
* modification, are permitted provided that the following conditions are met:
7+
* Redistributions of source code must retain the above copyright notice, this
8+
* list of conditions and the following disclaimer.
9+
*
10+
* Redistributions in binary form must reproduce the above copyright notice,
11+
* this list of conditions and the following disclaimer in the documentation
12+
* and/or other materials provided with the distribution.
13+
* Neither the name of the HISP project nor the names of its contributors may
14+
* be used to endorse or promote products derived from this software without
15+
* specific prior written permission.
16+
*
17+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
18+
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
19+
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
20+
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
21+
* ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
22+
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
23+
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
24+
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
25+
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
26+
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
27+
*/
28+
package org.hisp.dhis.user;
29+
30+
import org.hisp.dhis.common.UID;
31+
32+
/**
33+
* Event published after the authorities of a {@link UserRole} have changed, so that active sessions
34+
* of users with that role can be invalidated asynchronously, outside the updating transaction and
35+
* off the request thread.
36+
*
37+
* @author Morten Svanæs <msvanaes@dhis2.org>
38+
*/
39+
public record UserRoleAuthoritiesChangedEvent(UID userRoleUid) {}

dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/UserService.java

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -622,7 +622,20 @@ boolean canCurrentUserCanModify(
622622
@Nonnull
623623
List<UserLookup> getLinkedUserAccounts(@Nonnull User actingUser);
624624

625-
void invalidateUserSessions(String uid);
625+
/**
626+
* Invalidate all sessions for the given user.
627+
*
628+
* @param username the username of the user account.
629+
*/
630+
void invalidateUserSessions(String username);
631+
632+
/**
633+
* Returns the usernames of all users that are members of the user role with the given UID.
634+
*
635+
* @param roleUid the UID of the user role.
636+
* @return a list of usernames.
637+
*/
638+
List<String> getUsernamesByUserRole(@Nonnull UID roleUid);
626639

627640
/**
628641
* Register a account recovery attempt for the given user account.

dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/UserStore.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,14 @@ Map<String, Optional<Locale>> findNotifiableUsersWithPasswordLastUpdatedBetween(
161161
*/
162162
List<User> getUserByUsernames(Collection<String> usernames);
163163

164+
/**
165+
* Retrieves the usernames of all users that are members of the user role with the given UID.
166+
*
167+
* @param roleUid the UID of the user role.
168+
* @return a list of usernames.
169+
*/
170+
List<String> getUsernamesByUserRole(@Nonnull UID roleUid);
171+
164172
/**
165173
* Retrieves the most recently-used enabled User associated with the given open ID.
166174
*

dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/config/ServiceConfig.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.util.Map;
3232
import java.util.SortedMap;
3333
import java.util.TreeMap;
34+
import java.util.concurrent.Executor;
3435
import org.hisp.dhis.common.DeliveryChannel;
3536
import org.hisp.dhis.i18n.I18nManager;
3637
import org.hisp.dhis.i18n.ui.resourcebundle.DefaultResourceBundleManager;
@@ -45,6 +46,7 @@
4546
import org.hisp.dhis.user.UserSettingService;
4647
import org.springframework.context.annotation.Bean;
4748
import org.springframework.context.annotation.Configuration;
49+
import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor;
4850
import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
4951

5052
/**
@@ -93,4 +95,20 @@ public DefaultOutboundMessageBatchService defaultOutboundMessageBatchService(
9395
public ResourceBundleManager resourceBundleManager() {
9496
return new DefaultResourceBundleManager();
9597
}
98+
99+
/**
100+
* Single-threaded, bounded executor for asynchronous user session invalidation, see {@code
101+
* UserRoleSessionInvalidationListener}. A single thread serializes the work and keeps it from
102+
* competing with more important work.
103+
*/
104+
@Bean("userSessionInvalidationTaskExecutor")
105+
public Executor userSessionInvalidationTaskExecutor() {
106+
ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
107+
executor.setCorePoolSize(1);
108+
executor.setMaxPoolSize(1);
109+
executor.setQueueCapacity(1000);
110+
executor.setThreadNamePrefix("SessionInvalidation-");
111+
executor.initialize();
112+
return executor;
113+
}
96114
}

dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/user/DefaultUserService.java

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,13 +1088,41 @@ public List<UserLookup> getLinkedUserAccounts(@Nonnull User actingUser) {
10881088
}
10891089

10901090
@Override
1091-
public void invalidateUserSessions(String userUid) {
1092-
User user = getUser(userUid);
1093-
UserDetails userDetails = createUserDetails(user);
1094-
if (userDetails != null) {
1095-
List<SessionInformation> allSessions = sessionRegistry.getAllSessions(userDetails, false);
1096-
allSessions.forEach(SessionInformation::expireNow);
1091+
public void invalidateUserSessions(String username) {
1092+
if (username == null) {
1093+
return;
10971094
}
1095+
List<SessionInformation> sessions =
1096+
sessionRegistry.getAllSessions(sessionLookupPrincipal(username), false);
1097+
sessions.forEach(SessionInformation::expireNow);
1098+
}
1099+
1100+
/**
1101+
* Creates a minimal principal used only to look up sessions in the {@link SessionRegistry}. The
1102+
* in-memory registry matches principals by {@link UserDetailsImpl} equality, which includes the
1103+
* username only, and the Redis-backed registry resolves principals by name (username). Building
1104+
* this instead of loading the full user avoids one user fetch plus full {@code UserDetails}
1105+
* hydration (groups, roles, org units) per invalidated user.
1106+
*/
1107+
private static UserDetails sessionLookupPrincipal(String username) {
1108+
return UserDetailsImpl.builder()
1109+
.username(username)
1110+
.authorities(List.of())
1111+
.allAuthorities(Set.of())
1112+
.allRestrictions(Set.of())
1113+
.userSettings(Map.of())
1114+
.userGroupIds(Set.of())
1115+
.userOrgUnitIds(Set.of())
1116+
.userDataOrgUnitIds(Set.of())
1117+
.userSearchOrgUnitIds(Set.of())
1118+
.userRoleIds(Set.of())
1119+
.build();
1120+
}
1121+
1122+
@Override
1123+
@Transactional(readOnly = true)
1124+
public List<String> getUsernamesByUserRole(@Nonnull UID roleUid) {
1125+
return userStore.getUsernamesByUserRole(roleUid);
10981126
}
10991127

11001128
@Override
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/*
2+
* Copyright (c) 2004-2022, University of Oslo
3+
* All rights reserved.
4+
*
5+
* Redistribution and use in source and binary forms, with or without
6+
* modification, are permitted provided that the following conditions are met:
7+
* Redistributions of source code must retain the above copyright notice, this
8+
* list of conditions and the following disclaimer.
9+
*
10+
* Redistributions in binary form must reproduce the above copyright notice,
11+
* this list of conditions and the following disclaimer in the documentation
12+
* and/or other materials provided with the distribution.
13+
* Neither the name of the HISP project nor the names of its contributors may
14+
* be used to endorse or promote products derived from this software without
15+
* specific prior written permission.
16+
*
17+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
18+
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
19+
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
20+
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
21+
* ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
22+
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
23+
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
24+
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
25+
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
26+
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
27+
*/
28+
package org.hisp.dhis.user;
29+
30+
import com.google.common.collect.Lists;
31+
import java.util.List;
32+
import lombok.RequiredArgsConstructor;
33+
import lombok.extern.slf4j.Slf4j;
34+
import org.springframework.scheduling.annotation.Async;
35+
import org.springframework.stereotype.Component;
36+
import org.springframework.transaction.event.TransactionPhase;
37+
import org.springframework.transaction.event.TransactionalEventListener;
38+
39+
/**
40+
* Invalidates the sessions of all users that are members of a user role whose authorities have
41+
* changed, so that the new authorities take effect on the next authentication.
42+
*
43+
* <p>The work runs after the updating transaction has committed, on a dedicated single-threaded
44+
* executor, in batches with a pause in between. This keeps an authority change on a role with many
45+
* members cheap on the request thread and spreads the invalidation work out over time. If the
46+
* server shuts down before all batches have run, the remaining sessions simply keep the old
47+
* authorities until they expire or the user re-authenticates.
48+
*
49+
* @author Morten Svanæs <msvanaes@dhis2.org>
50+
*/
51+
@Slf4j
52+
@Component
53+
@RequiredArgsConstructor
54+
public class UserRoleSessionInvalidationListener {
55+
56+
/** Number of users whose sessions are invalidated per batch. */
57+
static final int BATCH_SIZE = 100;
58+
59+
/** Pause between batches, to spread the work out over time. */
60+
static final long BATCH_DELAY_MS = 500;
61+
62+
private final UserService userService;
63+
64+
@Async("userSessionInvalidationTaskExecutor")
65+
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT, fallbackExecution = true)
66+
public void onUserRoleAuthoritiesChanged(UserRoleAuthoritiesChangedEvent event) {
67+
List<String> usernames = userService.getUsernamesByUserRole(event.userRoleUid());
68+
log.info(
69+
"Invalidating sessions of {} users after authority change of user role '{}'",
70+
usernames.size(),
71+
event.userRoleUid());
72+
73+
List<List<String>> batches = Lists.partition(usernames, BATCH_SIZE);
74+
for (int i = 0; i < batches.size(); i++) {
75+
if (i > 0 && !pauseBetweenBatches()) {
76+
log.warn(
77+
"Session invalidation for user role '{}' interrupted, {} of {} batches done",
78+
event.userRoleUid(),
79+
i,
80+
batches.size());
81+
return;
82+
}
83+
batches.get(i).forEach(userService::invalidateUserSessions);
84+
}
85+
}
86+
87+
private static boolean pauseBetweenBatches() {
88+
try {
89+
Thread.sleep(BATCH_DELAY_MS);
90+
return true;
91+
} catch (InterruptedException e) {
92+
Thread.currentThread().interrupt();
93+
return false;
94+
}
95+
}
96+
}

dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/user/hibernate/HibernateUserStore.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,15 @@ public List<User> getUserByUsernames(Collection<String> usernames) {
603603
return query.getResultList();
604604
}
605605

606+
@Override
607+
public List<String> getUsernamesByUserRole(@Nonnull UID roleUid) {
608+
return getSession()
609+
.createQuery(
610+
"select u.username from User u join u.userRoles r where r.uid = :roleUid", String.class)
611+
.setParameter("roleUid", roleUid.getValue())
612+
.list();
613+
}
614+
606615
@Override
607616
public void setActiveLinkedAccounts(
608617
@Nonnull String actingUsername, @Nonnull String activeUsername) {
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
* Copyright (c) 2004-2022, University of Oslo
3+
* All rights reserved.
4+
*
5+
* Redistribution and use in source and binary forms, with or without
6+
* modification, are permitted provided that the following conditions are met:
7+
* Redistributions of source code must retain the above copyright notice, this
8+
* list of conditions and the following disclaimer.
9+
*
10+
* Redistributions in binary form must reproduce the above copyright notice,
11+
* this list of conditions and the following disclaimer in the documentation
12+
* and/or other materials provided with the distribution.
13+
* Neither the name of the HISP project nor the names of its contributors may
14+
* be used to endorse or promote products derived from this software without
15+
* specific prior written permission.
16+
*
17+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
18+
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
19+
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
20+
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
21+
* ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
22+
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
23+
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
24+
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
25+
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
26+
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
27+
*/
28+
package org.hisp.dhis.user;
29+
30+
import static org.mockito.ArgumentMatchers.anyString;
31+
import static org.mockito.Mockito.inOrder;
32+
import static org.mockito.Mockito.never;
33+
import static org.mockito.Mockito.times;
34+
import static org.mockito.Mockito.verify;
35+
import static org.mockito.Mockito.when;
36+
37+
import java.util.List;
38+
import java.util.stream.IntStream;
39+
import org.hisp.dhis.common.UID;
40+
import org.junit.jupiter.api.Test;
41+
import org.junit.jupiter.api.extension.ExtendWith;
42+
import org.mockito.InOrder;
43+
import org.mockito.InjectMocks;
44+
import org.mockito.Mock;
45+
import org.mockito.junit.jupiter.MockitoExtension;
46+
47+
/**
48+
* Unit test of {@link UserRoleSessionInvalidationListener}.
49+
*
50+
* @author Morten Svanæs <msvanaes@dhis2.org>
51+
*/
52+
@ExtendWith(MockitoExtension.class)
53+
class UserRoleSessionInvalidationListenerTest {
54+
55+
private static final UID ROLE_UID = UID.of("Rab1234abcd");
56+
57+
@Mock private UserService userService;
58+
59+
@InjectMocks private UserRoleSessionInvalidationListener listener;
60+
61+
@Test
62+
void invalidatesSessionsOfAllRoleMembersInOrder() {
63+
List<String> usernames = List.of("alice", "bob", "carol");
64+
when(userService.getUsernamesByUserRole(ROLE_UID)).thenReturn(usernames);
65+
66+
listener.onUserRoleAuthoritiesChanged(new UserRoleAuthoritiesChangedEvent(ROLE_UID));
67+
68+
InOrder order = inOrder(userService);
69+
usernames.forEach(username -> order.verify(userService).invalidateUserSessions(username));
70+
}
71+
72+
@Test
73+
void invalidatesSessionsOfAllRoleMembersAcrossBatches() {
74+
List<String> usernames =
75+
IntStream.range(0, UserRoleSessionInvalidationListener.BATCH_SIZE + 1)
76+
.mapToObj(i -> "user" + i)
77+
.toList();
78+
when(userService.getUsernamesByUserRole(ROLE_UID)).thenReturn(usernames);
79+
80+
listener.onUserRoleAuthoritiesChanged(new UserRoleAuthoritiesChangedEvent(ROLE_UID));
81+
82+
verify(userService, times(usernames.size())).invalidateUserSessions(anyString());
83+
}
84+
85+
@Test
86+
void invalidatesNothingWhenRoleHasNoMembers() {
87+
when(userService.getUsernamesByUserRole(ROLE_UID)).thenReturn(List.of());
88+
89+
listener.onUserRoleAuthoritiesChanged(new UserRoleAuthoritiesChangedEvent(ROLE_UID));
90+
91+
verify(userService, never()).invalidateUserSessions(anyString());
92+
}
93+
}

0 commit comments

Comments
 (0)