From 2a55c2ea7b5fc88d18786d6fc59ccf3b7c58683d Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Thu, 9 Jan 2025 16:32:44 -0500 Subject: [PATCH] Update comment in `additionalAuthenticationChecks` to clarify why our no-op implementation is ok --- .../hudson/security/AbstractPasswordBasedSecurityRealm.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/security/AbstractPasswordBasedSecurityRealm.java b/core/src/main/java/hudson/security/AbstractPasswordBasedSecurityRealm.java index b008697bbb88..5b7c362589cb 100644 --- a/core/src/main/java/hudson/security/AbstractPasswordBasedSecurityRealm.java +++ b/core/src/main/java/hudson/security/AbstractPasswordBasedSecurityRealm.java @@ -186,7 +186,10 @@ public GroupDetails loadGroupByGroupname(String groupname) throws org.acegisecur class Authenticator extends AbstractUserDetailsAuthenticationProvider { @Override protected void additionalAuthenticationChecks(UserDetails userDetails, UsernamePasswordAuthenticationToken authentication) throws AuthenticationException { - // authentication is assumed to be done already in the retrieveUser method + // Authentication is done in the retrieveUser method. Note that this method being a no-op is only safe + // because we use Spring Security's default NullUserCache. If caching was enabled, it would be possible to + // log in as any cached user with any password unless we updated this method to check the provided + // authentication as recommended in the superclass method's documentation, so be careful reusing this code. } @Override