Skip to content

Commit 823e25e

Browse files
devondragonclaude
devondragon
and
claude
committed
Fix AuthenticationEventListener to handle different authentication types properly
- Add support for DSUserDetails, OAuth2User, and String principals - Extract username/email from appropriate sources based on principal type - Handle OAuth2 authentication where email might be in attributes - Fall back gracefully when username cannot be extracted - Update tests to reflect proper behavior for edge cases - Ensure backward compatibility with existing authentication flows This fix addresses the issue where DSUserDetails had a null user in OAuth2 authentication scenarios by properly extracting the username from different principal types. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 89c06ef commit 823e25e

File tree

4 files changed

+420
-10
lines changed

4 files changed

+420
-10
lines changed

‎gradle.properties‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
version=3.2.4-SNAPSHOT
1+
version=3.3.0-SNAPSHOT
22
mavenCentralPublishing=true
33
mavenCentralAutomaticPublishing=true

‎src/main/java/com/digitalsanctuary/spring/user/listener/AuthenticationEventListener.java‎

Lines changed: 55 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
import org.springframework.context.event.EventListener;
44
import org.springframework.security.authentication.event.AbstractAuthenticationFailureEvent;
55
import org.springframework.security.authentication.event.AuthenticationSuccessEvent;
6+
import org.springframework.security.oauth2.core.user.OAuth2User;
67
import org.springframework.stereotype.Component;
8+
import com.digitalsanctuary.spring.user.service.DSUserDetails;
79
import com.digitalsanctuary.spring.user.service.LoginAttemptService;
810
import lombok.RequiredArgsConstructor;
911
import lombok.extern.slf4j.Slf4j;
@@ -22,28 +24,74 @@ public class AuthenticationEventListener {
2224

2325
/**
2426
* This method listens for successful authentications and handles account lockout functionality.
27+
* It properly handles different authentication types including form login, OAuth2, and OIDC.
2528
*
2629
* @param success the success event
2730
*/
2831
@EventListener
2932
public void onSuccess(AuthenticationSuccessEvent success) {
30-
// Handle successful authentication, e.g. logging or auditing
31-
log.debug("Authentication success: " + success.getAuthentication().getName());
32-
String username = success.getAuthentication().getName();
33-
loginAttemptService.loginSucceeded(username);
33+
// Extract username/email based on the principal type
34+
String username = null;
35+
Object principal = success.getAuthentication().getPrincipal();
36+
37+
if (principal instanceof DSUserDetails) {
38+
// Form login or custom authentication
39+
username = ((DSUserDetails) principal).getUsername();
40+
log.debug("Authentication success for DSUserDetails: {}", username);
41+
} else if (principal instanceof OAuth2User) {
42+
// OAuth2/OIDC authentication - try to get email
43+
OAuth2User oauth2User = (OAuth2User) principal;
44+
username = oauth2User.getAttribute("email");
45+
if (username == null) {
46+
// Fallback to name if email is not available
47+
username = oauth2User.getName();
48+
}
49+
log.debug("Authentication success for OAuth2User: {}", username);
50+
} else if (principal instanceof String) {
51+
// Basic authentication or remember-me
52+
username = (String) principal;
53+
log.debug("Authentication success for String principal: {}", username);
54+
} else {
55+
// Fallback to getName() method for unknown principal types
56+
username = success.getAuthentication().getName();
57+
log.debug("Authentication success for unknown principal type {}: {}",
58+
principal != null ? principal.getClass().getName() : "null", username);
59+
}
60+
61+
// Only process login success if we have a valid username
62+
if (username != null && !username.trim().isEmpty()) {
63+
loginAttemptService.loginSucceeded(username);
64+
} else {
65+
log.warn("Could not extract valid username from authentication event - not tracking");
66+
}
3467
}
3568

3669
/**
3770
* This method listens for authentication failures and handles account lockout functionality.
71+
* It properly handles different authentication types including form login, OAuth2, and OIDC.
3872
*
3973
* @param failure the failure event
4074
*/
4175
@EventListener
4276
public void onFailure(AbstractAuthenticationFailureEvent failure) {
43-
// Handle unsuccessful authentication, e.g. logging or auditing
44-
log.debug("Authentication failure: " + failure.getException().getMessage());
77+
// For failures, try to get username from getName() first (the attempted username)
4578
String username = failure.getAuthentication().getName();
46-
loginAttemptService.loginFailed(username);
79+
80+
// If getName() is null/empty, try to extract from principal
81+
if (username == null || username.trim().isEmpty()) {
82+
Object principal = failure.getAuthentication().getPrincipal();
83+
if (principal instanceof String) {
84+
username = (String) principal;
85+
}
86+
}
87+
88+
// Only process login failure if we have a valid username
89+
if (username != null && !username.trim().isEmpty()) {
90+
log.debug("Authentication failure for user '{}': {}", username, failure.getException().getMessage());
91+
loginAttemptService.loginFailed(username);
92+
} else {
93+
log.warn("Could not extract valid username from authentication failure event - not tracking");
94+
}
4795
}
4896

4997
}

‎src/test/java/com/digitalsanctuary/spring/user/listener/AuthenticationEventListenerOAuth2Test.java‎

Lines changed: 288 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,288 @@
1+
package com.digitalsanctuary.spring.user.listener;
2+
3+
import static org.mockito.ArgumentMatchers.anyString;
4+
import static org.mockito.ArgumentMatchers.eq;
5+
import static org.mockito.Mockito.never;
6+
import static org.mockito.Mockito.times;
7+
import static org.mockito.Mockito.verify;
8+
import static org.mockito.Mockito.when;
9+
10+
import java.util.Collection;
11+
import java.util.HashMap;
12+
import java.util.Map;
13+
14+
import org.junit.jupiter.api.BeforeEach;
15+
import org.junit.jupiter.api.Test;
16+
import org.junit.jupiter.api.extension.ExtendWith;
17+
import org.mockito.InjectMocks;
18+
import org.mockito.Mock;
19+
import org.mockito.junit.jupiter.MockitoExtension;
20+
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
21+
import org.springframework.security.authentication.event.AuthenticationSuccessEvent;
22+
import org.springframework.security.core.Authentication;
23+
import org.springframework.security.core.GrantedAuthority;
24+
import org.springframework.security.oauth2.core.user.OAuth2User;
25+
26+
import com.digitalsanctuary.spring.user.persistence.model.User;
27+
import com.digitalsanctuary.spring.user.service.DSUserDetails;
28+
import com.digitalsanctuary.spring.user.service.LoginAttemptService;
29+
30+
/**
31+
* Test class for AuthenticationEventListener OAuth2 authentication handling.
32+
* Verifies that the listener correctly extracts usernames from different principal types.
33+
*/
34+
@ExtendWith(MockitoExtension.class)
35+
class AuthenticationEventListenerOAuth2Test {
36+
37+
@Mock
38+
private LoginAttemptService loginAttemptService;
39+
40+
@Mock
41+
private OAuth2User oauth2User;
42+
43+
@Mock
44+
private DSUserDetails dsUserDetails;
45+
46+
@InjectMocks
47+
private AuthenticationEventListener authenticationEventListener;
48+
49+
private User testUser;
50+
51+
@BeforeEach
52+
void setUp() {
53+
testUser = new User();
54+
testUser.setEmail("[email protected]");
55+
testUser.setFirstName("Test");
56+
testUser.setLastName("User");
57+
}
58+
59+
@Test
60+
void onSuccess_withDSUserDetailsPrincipal_extractsEmailCorrectly() {
61+
// Given
62+
when(dsUserDetails.getUsername()).thenReturn("[email protected]");
63+
UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken(
64+
dsUserDetails, null, dsUserDetails.getAuthorities());
65+
AuthenticationSuccessEvent event = new AuthenticationSuccessEvent(auth);
66+
67+
// When
68+
authenticationEventListener.onSuccess(event);
69+
70+
// Then
71+
verify(loginAttemptService).loginSucceeded("[email protected]");
72+
}
73+
74+
@Test
75+
void onSuccess_withOAuth2UserPrincipalWithEmail_extractsEmailCorrectly() {
76+
// Given
77+
when(oauth2User.getAttribute("email")).thenReturn("[email protected]");
78+
79+
UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken(
80+
oauth2User, null, oauth2User.getAuthorities());
81+
AuthenticationSuccessEvent event = new AuthenticationSuccessEvent(auth);
82+
83+
// When
84+
authenticationEventListener.onSuccess(event);
85+
86+
// Then
87+
verify(loginAttemptService).loginSucceeded("[email protected]");
88+
}
89+
90+
@Test
91+
void onSuccess_withOAuth2UserPrincipalWithoutEmail_fallsBackToName() {
92+
// Given
93+
when(oauth2User.getAttribute("email")).thenReturn(null);
94+
when(oauth2User.getName()).thenReturn("oauth_user_123");
95+
96+
UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken(
97+
oauth2User, null, oauth2User.getAuthorities());
98+
AuthenticationSuccessEvent event = new AuthenticationSuccessEvent(auth);
99+
100+
// When
101+
authenticationEventListener.onSuccess(event);
102+
103+
// Then
104+
verify(loginAttemptService).loginSucceeded("oauth_user_123");
105+
}
106+
107+
@Test
108+
void onSuccess_withStringPrincipal_extractsUsernameCorrectly() {
109+
// Given
110+
String username = "[email protected]";
111+
UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken(
112+
username, null);
113+
AuthenticationSuccessEvent event = new AuthenticationSuccessEvent(auth);
114+
115+
// When
116+
authenticationEventListener.onSuccess(event);
117+
118+
// Then
119+
verify(loginAttemptService).loginSucceeded(username);
120+
}
121+
122+
@Test
123+
void onSuccess_withUnknownPrincipalType_fallsBackToGetName() {
124+
// Given
125+
Object unknownPrincipal = new Object() {
126+
@Override
127+
public String toString() {
128+
return "unknown_principal";
129+
}
130+
};
131+
132+
UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken(
133+
unknownPrincipal, null);
134+
AuthenticationSuccessEvent event = new AuthenticationSuccessEvent(auth) {
135+
@Override
136+
public Authentication getAuthentication() {
137+
return new Authentication() {
138+
@Override
139+
public String getName() {
140+
return "[email protected]";
141+
}
142+
143+
@Override
144+
public Collection<? extends GrantedAuthority> getAuthorities() {
145+
return auth.getAuthorities();
146+
}
147+
148+
@Override
149+
public Object getCredentials() {
150+
return auth.getCredentials();
151+
}
152+
153+
@Override
154+
public Object getDetails() {
155+
return auth.getDetails();
156+
}
157+
158+
@Override
159+
public Object getPrincipal() {
160+
return unknownPrincipal;
161+
}
162+
163+
@Override
164+
public boolean isAuthenticated() {
165+
return auth.isAuthenticated();
166+
}
167+
168+
@Override
169+
public void setAuthenticated(boolean isAuthenticated) throws IllegalArgumentException {
170+
auth.setAuthenticated(isAuthenticated);
171+
}
172+
};
173+
}
174+
};
175+
176+
// When
177+
authenticationEventListener.onSuccess(event);
178+
179+
// Then
180+
verify(loginAttemptService).loginSucceeded("[email protected]");
181+
}
182+
183+
@Test
184+
void onSuccess_withNullUsername_doesNotCallLoginAttemptService() {
185+
// Given
186+
when(oauth2User.getAttribute("email")).thenReturn(null);
187+
when(oauth2User.getName()).thenReturn(null);
188+
189+
// Create an authentication with a custom implementation that returns null for getName()
190+
Authentication auth = new Authentication() {
191+
@Override
192+
public String getName() {
193+
return null;
194+
}
195+
196+
@Override
197+
public Collection<? extends GrantedAuthority> getAuthorities() {
198+
return oauth2User.getAuthorities();
199+
}
200+
201+
@Override
202+
public Object getCredentials() {
203+
return null;
204+
}
205+
206+
@Override
207+
public Object getDetails() {
208+
return null;
209+
}
210+
211+
@Override
212+
public Object getPrincipal() {
213+
return oauth2User;
214+
}
215+
216+
@Override
217+
public boolean isAuthenticated() {
218+
return true;
219+
}
220+
221+
@Override
222+
public void setAuthenticated(boolean isAuthenticated) throws IllegalArgumentException {
223+
}
224+
};
225+
226+
AuthenticationSuccessEvent event = new AuthenticationSuccessEvent(auth);
227+
228+
// When
229+
authenticationEventListener.onSuccess(event);
230+
231+
// Then
232+
verify(loginAttemptService, never()).loginSucceeded(anyString());
233+
}
234+
235+
@Test
236+
void onSuccess_withNullPrincipal_handlesGracefully() {
237+
// Given
238+
UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken(
239+
null, null);
240+
AuthenticationSuccessEvent event = new AuthenticationSuccessEvent(auth) {
241+
@Override
242+
public Authentication getAuthentication() {
243+
return new Authentication() {
244+
@Override
245+
public String getName() {
246+
return "[email protected]";
247+
}
248+
249+
@Override
250+
public Collection<? extends GrantedAuthority> getAuthorities() {
251+
return auth.getAuthorities();
252+
}
253+
254+
@Override
255+
public Object getCredentials() {
256+
return auth.getCredentials();
257+
}
258+
259+
@Override
260+
public Object getDetails() {
261+
return auth.getDetails();
262+
}
263+
264+
@Override
265+
public Object getPrincipal() {
266+
return null;
267+
}
268+
269+
@Override
270+
public boolean isAuthenticated() {
271+
return auth.isAuthenticated();
272+
}
273+
274+
@Override
275+
public void setAuthenticated(boolean isAuthenticated) throws IllegalArgumentException {
276+
auth.setAuthenticated(isAuthenticated);
277+
}
278+
};
279+
}
280+
};
281+
282+
// When
283+
authenticationEventListener.onSuccess(event);
284+
285+
// Then
286+
verify(loginAttemptService).loginSucceeded("[email protected]");
287+
}
288+
}

0 commit comments

Comments
 (0)