Skip to content

Commit 50085ea

Browse files
jlangyjlangy
authored
Merge pull request #376 from bcgov/ssoteam-1848
fix: user-session remover
2 parents ee8f1ba + dcb5932 commit 50085ea

File tree

5 files changed

+183
-21
lines changed

5 files changed

+183
-21
lines changed

‎.tool-versions‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@ k6 0.34.1
88
terraform 1.2.0
99
terraform-docs 0.12.1
1010
tflint 0.28.1
11-
java openjdk-14.0.1
11+
java openjdk-17.0.1
1212
gradle 7.3.1

‎docker/keycloak/configuration/24/quarkus.properties‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ quarkus.log.file.json.exception-output-type=formatted
55
quarkus.log.file.json.key-overrides=timestamp=@timestamp
66
quarkus.log.file.json.additional-field."@version".value=1
77
# Quarkus will auto-compress if ending with .zip: https://quarkus.io/guides/logging.
8-
quarkus.log.file.rotation.file-suffix=.zip
8+
quarkus.log.file.rotation.file-suffix=${QUARKUS_LOG_FILE_ROTATION_FILE_SUFFIX:.zip}
99
# Optional: Disable rotation by size (adjust value as needed)
10-
quarkus.log.file.rotation.max-file-size=200M
11-
# The number of rotated files. From above configuration, this will keep 200M * 42 files ~= 8Gigabytes of data before replacing.
12-
quarkus.log.file.rotation.max-backup-index=42
10+
quarkus.log.file.rotation.max-file-size=${QUARKUS_LOG_FILE_ROTATION_MAX_FILE_SIZE:200M}
11+
# The number of rotated files per pod. From above configuration, this will keep 200M * 14 files * 3pods ~= 8Gigabytes of data before replacing.
12+
quarkus.log.file.rotation.max-backup-index=${QUARKUS_LOG_FILE_ROTATION_MAX_BACKUP_INDEX:14}

‎docker/keycloak/extensions-24/services/pom.xml‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@
4646
<target>17</target>
4747
</configuration>
4848
</plugin>
49+
<plugin>
50+
<groupId>org.apache.maven.plugins</groupId>
51+
<artifactId>maven-surefire-plugin</artifactId>
52+
<version>2.22.0</version>
53+
</plugin>
4954
</plugins>
5055
</build>
5156

@@ -136,18 +141,31 @@
136141
<dependency>
137142
<groupId>junit</groupId>
138143
<artifactId>junit</artifactId>
144+
<version>4.13.2</version>
139145
<scope>test</scope>
140146
</dependency>
141147
<dependency>
142148
<groupId>org.mockito</groupId>
143-
<artifactId>mockito-all</artifactId>
144-
<version>1.9.5</version>
149+
<artifactId>mockito-core</artifactId>
150+
<version>5.3.1</version>
145151
<scope>test</scope>
146152
</dependency>
147153
<dependency>
148154
<groupId>org.hamcrest</groupId>
149155
<artifactId>hamcrest-all</artifactId>
150156
<scope>test</scope>
151157
</dependency>
158+
<dependency>
159+
<groupId>org.junit.jupiter</groupId>
160+
<artifactId>junit-jupiter-engine</artifactId>
161+
<version>5.9.1</version>
162+
<scope>test</scope>
163+
</dependency>
164+
<dependency>
165+
<groupId>org.junit.jupiter</groupId>
166+
<artifactId>junit-jupiter-api</artifactId>
167+
<version>5.9.1</version>
168+
<scope>test</scope>
169+
</dependency>
152170
</dependencies>
153171
</project>

‎docker/keycloak/extensions-24/services/src/main/java/com/github/bcgov/keycloak/authenticators/UserSessionRemover.java‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66
import org.keycloak.models.RealmModel;
77
import org.keycloak.models.UserModel;
88
import org.keycloak.models.UserSessionProvider;
9+
import org.keycloak.models.AuthenticatedClientSessionModel;
910
import org.keycloak.services.managers.AuthenticationManager;
1011
import org.keycloak.authentication.AuthenticationFlowContext;
1112
import org.keycloak.sessions.AuthenticationSessionModel;
13+
import org.keycloak.models.UserSessionModel;
1214

1315
import java.util.Map;
1416

@@ -23,34 +25,31 @@ public boolean requiresUser() {
2325

2426
@Override
2527
public void authenticate(AuthenticationFlowContext context) {
26-
AuthenticationSessionModel session = context.getAuthenticationSession();
27-
AuthenticationManager.AuthResult authResult = AuthenticationManager.authenticateIdentityCookie(
28-
context.getSession(),
29-
context.getRealm(),
30-
true
31-
);
28+
UserSessionModel userSessionModel;
29+
AuthenticationManager.AuthResult authResult = AuthenticationManager.authenticateIdentityCookie(context.getSession(), context.getRealm(), true);
3230

3331
// 1. If no Cookie session, proceed to next step
3432
if (authResult == null) {
3533
context.attempted();
3634
return;
3735
}
3836

39-
// Need to use the KeycloakSession context to get the authenticating client ID. Not available on the AuthenticationFlowContext.
40-
KeycloakSession keycloakSession = context.getSession();
41-
String authenticatingClientUUID = keycloakSession.getContext().getClient().getId();
37+
userSessionModel = authResult.getSession();
4238

43-
// Get all existing sessions. If any session is associated with a different client, clear all user sessions.
44-
UserSessionProvider userSessionProvider = keycloakSession.sessions();
45-
Map<String, Long> activeClientSessionStats = userSessionProvider.getActiveClientSessionStats(context.getRealm(), false);
39+
String authenticatingClientUUID = context.getSession().getContext().getClient().getId();
40+
UserSessionProvider userSessionProvider = context.getSession().sessions();
4641

47-
for (String activeSessionClientUUID : activeClientSessionStats.keySet()) {
42+
// Must fetch sessions from the user session model, user session provider has all session in the realm
43+
Map<String, AuthenticatedClientSessionModel> authenticatedClientSessions = userSessionModel.getAuthenticatedClientSessions();
44+
45+
for (String activeSessionClientUUID : authenticatedClientSessions.keySet()) {
4846
if (!activeSessionClientUUID.equals(authenticatingClientUUID)) {
49-
userSessionProvider.removeUserSession(context.getRealm(), authResult.getSession());
47+
userSessionProvider.removeUserSession(context.getRealm(), userSessionModel);
5048
}
5149
}
5250

5351
context.attempted();
52+
return;
5453
}
5554

5655
@Override

‎docker/keycloak/extensions-24/services/src/test/java/com/github/bcgov/keycloak/authenticators/UserSessionRemoverTest.java‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
package com.github.bcgov.keycloak.testsuite.authenticators;
2+
3+
import static org.mockito.Mockito.mock;
4+
import static org.mockito.Mockito.when;
5+
import static org.mockito.ArgumentMatchers.any;
6+
import static org.mockito.Mockito.times;
7+
import static org.mockito.Mockito.verify;
8+
9+
import org.mockito.Mockito;
10+
import org.mockito.MockedStatic;
11+
import org.junit.jupiter.api.Test;
12+
import org.junit.jupiter.api.BeforeEach;
13+
14+
import com.github.bcgov.keycloak.authenticators.UserSessionRemover;
15+
import org.keycloak.authentication.AuthenticationFlowContext;
16+
import org.keycloak.models.KeycloakSession;
17+
import org.keycloak.models.AuthenticatedClientSessionModel;
18+
import org.keycloak.models.RealmModel;
19+
import org.keycloak.services.managers.AuthenticationManager;
20+
import org.keycloak.models.ClientModel;
21+
import org.keycloak.sessions.AuthenticationSessionModel;
22+
import org.keycloak.models.UserSessionProvider;
23+
import org.keycloak.models.AuthenticatedClientSessionModel;
24+
import org.keycloak.models.UserSessionModel;
25+
import org.keycloak.models.KeycloakContext;
26+
import java.util.HashMap;
27+
import java.util.Map;
28+
29+
public class UserSessionRemoverTest {
30+
private static final UserSessionRemover userSessionRemover = new UserSessionRemover();
31+
32+
private AuthenticationFlowContext context;
33+
private KeycloakSession session;
34+
private RealmModel realm;
35+
private AuthenticationSessionModel authSession;
36+
private UserSessionProvider userSessionProvider;
37+
private KeycloakSession keycloakSession;
38+
private ClientModel client;
39+
private KeycloakContext keycloakContext;
40+
private AuthenticationManager.AuthResult authResult;
41+
private UserSessionModel userSessionModel;
42+
private AuthenticatedClientSessionModel authenticatedClientSessionModel;
43+
44+
@BeforeEach
45+
public void setup() {
46+
// Initialize mocks for necessary objects
47+
context = mock(AuthenticationFlowContext.class);
48+
realm = mock(RealmModel.class);
49+
authSession = mock(AuthenticationSessionModel.class);
50+
userSessionProvider = mock(UserSessionProvider.class);
51+
keycloakSession = mock(KeycloakSession.class);
52+
keycloakContext = mock(KeycloakContext.class);
53+
client = mock(ClientModel.class);
54+
authResult = mock(AuthenticationManager.AuthResult.class);
55+
userSessionModel = mock(UserSessionModel.class);
56+
authenticatedClientSessionModel = mock(AuthenticatedClientSessionModel.class);
57+
58+
59+
// Set up common behavior of the mocks
60+
when(context.getSession()).thenReturn(keycloakSession);
61+
when(context.getRealm()).thenReturn(realm);
62+
when(context.getAuthenticationSession()).thenReturn(authSession);
63+
when(keycloakSession.sessions()).thenReturn(userSessionProvider);
64+
when(context.getSession()).thenReturn(keycloakSession);
65+
when(keycloakSession.getContext()).thenReturn(keycloakContext);
66+
when(keycloakContext.getClient()).thenReturn(client);
67+
when(authResult.getSession()).thenReturn(userSessionModel);
68+
}
69+
70+
@Test
71+
public void testSkipClientSessionCheckWhenNullAuthResult() throws Exception {
72+
try (MockedStatic<AuthenticationManager> authenticationManager = Mockito.mockStatic(AuthenticationManager.class)) {
73+
authenticationManager.when(() -> AuthenticationManager.authenticateIdentityCookie(
74+
any(KeycloakSession.class), any(RealmModel.class), any(Boolean.class)
75+
)).thenReturn(null);
76+
userSessionRemover.authenticate(context);
77+
78+
// Keycloak Session Context check skipped if no Auth Session
79+
verify(keycloakSession, times(0)).getContext();
80+
verify(userSessionProvider, times(0)).removeUserSession(any(RealmModel.class), any(UserSessionModel.class));
81+
}
82+
}
83+
84+
@Test
85+
public void testRemovesUserSessionsWhenMultipleClientSessionsExist() throws Exception {
86+
when(client.getId()).thenReturn("client1");
87+
Map<String, AuthenticatedClientSessionModel> authenticatedClientSessions = new HashMap<>();
88+
authenticatedClientSessions.put("client1", authenticatedClientSessionModel);
89+
authenticatedClientSessions.put("client2", authenticatedClientSessionModel);
90+
91+
when(userSessionModel.getAuthenticatedClientSessions()).thenReturn(authenticatedClientSessions);
92+
93+
try (MockedStatic<AuthenticationManager> authenticationManager = Mockito.mockStatic(AuthenticationManager.class)) {
94+
authenticationManager.when(() -> AuthenticationManager.authenticateIdentityCookie(
95+
any(KeycloakSession.class), any(RealmModel.class), any(Boolean.class)
96+
)).thenReturn(authResult);
97+
98+
userSessionRemover.authenticate(context);
99+
100+
verify(keycloakSession, times(1)).getContext();
101+
verify(userSessionProvider, times(1)).removeUserSession(any(RealmModel.class), any(UserSessionModel.class));
102+
}
103+
}
104+
105+
@Test
106+
public void testRemovesUserSessionsWhenSingleDifferentClientSessionFound() throws Exception {
107+
when(client.getId()).thenReturn("client1");
108+
Map<String, AuthenticatedClientSessionModel> authenticatedClientSessions = new HashMap<>();
109+
authenticatedClientSessions.put("client2", authenticatedClientSessionModel);
110+
111+
when(userSessionModel.getAuthenticatedClientSessions()).thenReturn(authenticatedClientSessions);
112+
113+
try (MockedStatic<AuthenticationManager> authenticationManager = Mockito.mockStatic(AuthenticationManager.class)) {
114+
authenticationManager.when(() -> AuthenticationManager.authenticateIdentityCookie(
115+
any(KeycloakSession.class), any(RealmModel.class), any(Boolean.class)
116+
)).thenReturn(authResult);
117+
userSessionRemover.authenticate(context);
118+
119+
verify(keycloakSession, times(1)).getContext();
120+
verify(userSessionProvider, times(1)).removeUserSession(any(RealmModel.class), any(UserSessionModel.class));
121+
}
122+
}
123+
124+
@Test
125+
public void testLeavesExistingSessionWhenOnlyAssociatedToAuthenticatingClient() throws Exception {
126+
when(client.getId()).thenReturn("client1");
127+
Map<String, AuthenticatedClientSessionModel> authenticatedClientSessions = new HashMap<>();
128+
authenticatedClientSessions.put("client1", authenticatedClientSessionModel);
129+
130+
when(userSessionModel.getAuthenticatedClientSessions()).thenReturn(authenticatedClientSessions);
131+
132+
try (MockedStatic<AuthenticationManager> authenticationManager = Mockito.mockStatic(AuthenticationManager.class)) {
133+
authenticationManager.when(() -> AuthenticationManager.authenticateIdentityCookie(
134+
any(KeycloakSession.class), any(RealmModel.class), any(Boolean.class)
135+
)).thenReturn(authResult);
136+
userSessionRemover.authenticate(context);
137+
138+
// Verify the keycloak session context is invoked to check client sessions
139+
verify(keycloakSession, times(1)).getContext();
140+
141+
// Remove user session should be skipped
142+
verify(userSessionProvider, times(0)).removeUserSession(any(RealmModel.class), any(UserSessionModel.class));
143+
}
144+
}
145+
}

0 commit comments

Comments
 (0)