Skip to content

Commit fa7baf5

Browse files
jzheauxjzheaux
committed
Restructure Logs
Followed common use cases based off of HelloWorld sample: - Public endpoint - Unauthorized endpoint - Undefined endpoint - Successful form login - Failed form login - Post-login redirect Issue gh-6311
1 parent 17f1540 commit fa7baf5

File tree

54 files changed

+386
-211
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

54 files changed

+386
-211
lines changed

‎config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/AuthenticationConfiguration.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,9 @@ private static class EnableGlobalAuthenticationAutowiredConfigurer extends Globa
212212
public void init(AuthenticationManagerBuilder auth) {
213213
Map<String, Object> beansWithAnnotation = this.context
214214
.getBeansWithAnnotation(EnableGlobalAuthentication.class);
215-
logger.debug(LogMessage.format("Eagerly initializing %s", beansWithAnnotation));
215+
if (logger.isTraceEnabled()) {
216+
logger.trace(LogMessage.format("Eagerly initializing %s", beansWithAnnotation));
217+
}
216218
}
217219

218220
}

‎core/src/main/java/org/springframework/security/access/intercept/AbstractSecurityInterceptor.java

+36-10
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,13 @@ private void validateAttributeDefs(Collection<ConfigAttribute> attributeDefs) {
167167
}
168168
}
169169
if (unsupportedAttrs.size() != 0) {
170+
this.logger
171+
.trace("Did not validate configuration attributes since validateConfigurationAttributes is false");
170172
throw new IllegalArgumentException("Unsupported configuration attributes: " + unsupportedAttrs);
171173
}
172-
this.logger.debug("Validated configuration attributes");
174+
else {
175+
this.logger.trace("Validated configuration attributes");
176+
}
173177
}
174178

175179
protected InterceptorStatusToken beforeInvocation(Object object) {
@@ -186,34 +190,43 @@ protected InterceptorStatusToken beforeInvocation(Object object) {
186190
+ " was denied as public invocations are not allowed via this interceptor. "
187191
+ "This indicates a configuration error because the "
188192
+ "rejectPublicInvocations property is set to 'true'");
189-
this.logger.debug("Public object - authentication not attempted");
193+
if (this.logger.isDebugEnabled()) {
194+
this.logger.debug(LogMessage.format("Authorized public object %s", object));
195+
}
190196
publishEvent(new PublicInvocationEvent(object));
191197
return null; // no further work post-invocation
192198
}
193-
this.logger.debug(LogMessage.format("Secure object: %s; Attributes: %s", object, attributes));
194199
if (SecurityContextHolder.getContext().getAuthentication() == null) {
195200
credentialsNotFound(this.messages.getMessage("AbstractSecurityInterceptor.authenticationNotFound",
196201
"An Authentication object was not found in the SecurityContext"), object, attributes);
197202
}
198203
Authentication authenticated = authenticateIfRequired();
204+
if (this.logger.isTraceEnabled()) {
205+
this.logger.trace(LogMessage.format("Authorizing %s with attributes %s", object, attributes));
206+
}
199207
// Attempt authorization
200208
attemptAuthorization(object, attributes, authenticated);
201-
this.logger.debug("Authorization successful");
209+
if (this.logger.isDebugEnabled()) {
210+
this.logger.debug(LogMessage.format("Authorized %s with attributes %s", object, attributes));
211+
}
202212
if (this.publishAuthorizationSuccess) {
203213
publishEvent(new AuthorizedEvent(object, attributes, authenticated));
204214
}
205215

206216
// Attempt to run as a different user
207217
Authentication runAs = this.runAsManager.buildRunAs(authenticated, object, attributes);
208218
if (runAs != null) {
209-
this.logger.debug(LogMessage.format("Switching to RunAs Authentication: %s", runAs));
210219
SecurityContext origCtx = SecurityContextHolder.getContext();
211220
SecurityContextHolder.setContext(SecurityContextHolder.createEmptyContext());
212221
SecurityContextHolder.getContext().setAuthentication(runAs);
222+
223+
if (this.logger.isDebugEnabled()) {
224+
this.logger.debug(LogMessage.format("Switched to RunAs authentication %s", runAs));
225+
}
213226
// need to revert to token.Authenticated post-invocation
214227
return new InterceptorStatusToken(origCtx, true, attributes, object);
215228
}
216-
this.logger.debug("RunAsManager did not change Authentication object");
229+
this.logger.trace("Did not switch RunAs authentication since RunAsManager returned null");
217230
// no further work post-invocation
218231
return new InterceptorStatusToken(SecurityContextHolder.getContext(), false, attributes, object);
219232

@@ -225,6 +238,13 @@ private void attemptAuthorization(Object object, Collection<ConfigAttribute> att
225238
this.accessDecisionManager.decide(authenticated, object, attributes);
226239
}
227240
catch (AccessDeniedException ex) {
241+
if (this.logger.isTraceEnabled()) {
242+
this.logger.trace(LogMessage.format("Failed to authorize %s with attributes %s using %s", object,
243+
attributes, this.accessDecisionManager));
244+
}
245+
else if (this.logger.isDebugEnabled()) {
246+
this.logger.debug(LogMessage.format("Failed to authorize %s with attributes %s", object, attributes));
247+
}
228248
publishEvent(new AuthorizationFailureEvent(object, attributes, authenticated, ex));
229249
throw ex;
230250
}
@@ -239,9 +259,11 @@ private void attemptAuthorization(Object object, Collection<ConfigAttribute> att
239259
*/
240260
protected void finallyInvocation(InterceptorStatusToken token) {
241261
if (token != null && token.isContextHolderRefreshRequired()) {
242-
this.logger.debug(LogMessage.of(
243-
() -> "Reverting to original Authentication: " + token.getSecurityContext().getAuthentication()));
244262
SecurityContextHolder.setContext(token.getSecurityContext());
263+
if (this.logger.isDebugEnabled()) {
264+
this.logger.debug(LogMessage.of(
265+
() -> "Reverted to original authentication " + token.getSecurityContext().getAuthentication()));
266+
}
245267
}
246268
}
247269

@@ -284,12 +306,16 @@ protected Object afterInvocation(InterceptorStatusToken token, Object returnedOb
284306
private Authentication authenticateIfRequired() {
285307
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
286308
if (authentication.isAuthenticated() && !this.alwaysReauthenticate) {
287-
this.logger.debug(LogMessage.format("Previously Authenticated: %s", authentication));
309+
if (this.logger.isTraceEnabled()) {
310+
this.logger.trace(LogMessage.format("Did not re-authenticate %s before authorizing", authentication));
311+
}
288312
return authentication;
289313
}
290314
authentication = this.authenticationManager.authenticate(authentication);
291315
// Don't authenticated.setAuthentication(true) because each provider does that
292-
this.logger.debug(LogMessage.format("Successfully Authenticated: %s", authentication));
316+
if (this.logger.isDebugEnabled()) {
317+
this.logger.debug(LogMessage.format("Re-authenticated %s before authorizing", authentication));
318+
}
293319
SecurityContextHolder.getContext().setAuthentication(authentication);
294320
return authentication;
295321
}

‎core/src/main/java/org/springframework/security/access/vote/AbstractAccessDecisionManager.java

+6
Original file line numberDiff line numberDiff line change
@@ -115,4 +115,10 @@ public boolean supports(Class<?> clazz) {
115115
return true;
116116
}
117117

118+
@Override
119+
public String toString() {
120+
return this.getClass().getSimpleName() + " [DecisionVoters=" + this.decisionVoters
121+
+ ", AllowIfAllAbstainDecisions=" + this.allowIfAllAbstainDecisions + "]";
122+
}
123+
118124
}

‎core/src/main/java/org/springframework/security/access/vote/AffirmativeBased.java

-2
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import java.util.Collection;
2020
import java.util.List;
2121

22-
import org.springframework.core.log.LogMessage;
2322
import org.springframework.security.access.AccessDecisionVoter;
2423
import org.springframework.security.access.AccessDeniedException;
2524
import org.springframework.security.access.ConfigAttribute;
@@ -59,7 +58,6 @@ public void decide(Authentication authentication, Object object, Collection<Conf
5958
int deny = 0;
6059
for (AccessDecisionVoter voter : getDecisionVoters()) {
6160
int result = voter.vote(authentication, object, configAttributes);
62-
this.logger.debug(LogMessage.format("Voter: %s, returned: %s", voter, result));
6361
switch (result) {
6462
case AccessDecisionVoter.ACCESS_GRANTED:
6563
return;

‎core/src/main/java/org/springframework/security/access/vote/ConsensusBased.java

-2
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import java.util.Collection;
2020
import java.util.List;
2121

22-
import org.springframework.core.log.LogMessage;
2322
import org.springframework.security.access.AccessDecisionVoter;
2423
import org.springframework.security.access.AccessDeniedException;
2524
import org.springframework.security.access.ConfigAttribute;
@@ -67,7 +66,6 @@ public void decide(Authentication authentication, Object object, Collection<Conf
6766
int deny = 0;
6867
for (AccessDecisionVoter voter : getDecisionVoters()) {
6968
int result = voter.vote(authentication, object, configAttributes);
70-
this.logger.debug(LogMessage.format("Voter: %s, returned: %s", voter, result));
7169
switch (result) {
7270
case AccessDecisionVoter.ACCESS_GRANTED:
7371
grant++;

‎core/src/main/java/org/springframework/security/access/vote/UnanimousBased.java

-2
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import java.util.Collection;
2121
import java.util.List;
2222

23-
import org.springframework.core.log.LogMessage;
2423
import org.springframework.security.access.AccessDecisionVoter;
2524
import org.springframework.security.access.AccessDeniedException;
2625
import org.springframework.security.access.ConfigAttribute;
@@ -67,7 +66,6 @@ public void decide(Authentication authentication, Object object, Collection<Conf
6766
singleAttributeList.set(0, attribute);
6867
for (AccessDecisionVoter voter : getDecisionVoters()) {
6968
int result = voter.vote(authentication, object, singleAttributeList);
70-
this.logger.debug(LogMessage.format("Voter: %s, returned: %s", voter, result));
7169
switch (result) {
7270
case AccessDecisionVoter.ACCESS_GRANTED:
7371
grant++;

‎core/src/main/java/org/springframework/security/authentication/AbstractAuthenticationToken.java

+7-18
Original file line numberDiff line numberDiff line change
@@ -174,24 +174,13 @@ public int hashCode() {
174174
@Override
175175
public String toString() {
176176
StringBuilder sb = new StringBuilder();
177-
sb.append(super.toString()).append(": ");
178-
sb.append("Principal: ").append(this.getPrincipal()).append("; ");
179-
sb.append("Credentials: [PROTECTED]; ");
180-
sb.append("Authenticated: ").append(this.isAuthenticated()).append("; ");
181-
sb.append("Details: ").append(this.getDetails()).append("; ");
182-
if (!this.authorities.isEmpty()) {
183-
sb.append("Granted Authorities: ");
184-
int i = 0;
185-
for (GrantedAuthority authority : this.authorities) {
186-
if (i++ > 0) {
187-
sb.append(", ");
188-
}
189-
sb.append(authority);
190-
}
191-
}
192-
else {
193-
sb.append("Not granted any authorities");
194-
}
177+
sb.append(getClass().getSimpleName()).append(" [");
178+
sb.append("Principal=").append(getPrincipal()).append(", ");
179+
sb.append("Credentials=[PROTECTED], ");
180+
sb.append("Authenticated=").append(isAuthenticated()).append(", ");
181+
sb.append("Details=").append(getDetails()).append(", ");
182+
sb.append("Granted Authorities=").append(this.authorities);
183+
sb.append("]");
195184
return sb.toString();
196185
}
197186

‎core/src/main/java/org/springframework/security/authentication/AccountStatusUserDetailsChecker.java

+9
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616

1717
package org.springframework.security.authentication;
1818

19+
import org.apache.commons.logging.Log;
20+
import org.apache.commons.logging.LogFactory;
21+
1922
import org.springframework.context.MessageSource;
2023
import org.springframework.context.MessageSourceAware;
2124
import org.springframework.context.support.MessageSourceAccessor;
@@ -29,23 +32,29 @@
2932
*/
3033
public class AccountStatusUserDetailsChecker implements UserDetailsChecker, MessageSourceAware {
3134

35+
private final Log logger = LogFactory.getLog(getClass());
36+
3237
protected MessageSourceAccessor messages = SpringSecurityMessageSource.getAccessor();
3338

3439
@Override
3540
public void check(UserDetails user) {
3641
if (!user.isAccountNonLocked()) {
42+
this.logger.debug("Failed to authenticate since user account is locked");
3743
throw new LockedException(
3844
this.messages.getMessage("AccountStatusUserDetailsChecker.locked", "User account is locked"));
3945
}
4046
if (!user.isEnabled()) {
47+
this.logger.debug("Failed to authenticate since user account is disabled");
4148
throw new DisabledException(
4249
this.messages.getMessage("AccountStatusUserDetailsChecker.disabled", "User is disabled"));
4350
}
4451
if (!user.isAccountNonExpired()) {
52+
this.logger.debug("Failed to authenticate since user account is expired");
4553
throw new AccountExpiredException(
4654
this.messages.getMessage("AccountStatusUserDetailsChecker.expired", "User account has expired"));
4755
}
4856
if (!user.isCredentialsNonExpired()) {
57+
this.logger.debug("Failed to authenticate since user account credentials have expired");
4958
throw new CredentialsExpiredException(this.messages
5059
.getMessage("AccountStatusUserDetailsChecker.credentialsExpired", "User credentials have expired"));
5160
}

‎core/src/main/java/org/springframework/security/authentication/ProviderManager.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -168,11 +168,16 @@ public Authentication authenticate(Authentication authentication) throws Authent
168168
AuthenticationException parentException = null;
169169
Authentication result = null;
170170
Authentication parentResult = null;
171+
int currentPosition = 0;
172+
int size = this.providers.size();
171173
for (AuthenticationProvider provider : getProviders()) {
172174
if (!provider.supports(toTest)) {
173175
continue;
174176
}
175-
logger.debug(LogMessage.format("Authentication attempt using %s", provider.getClass().getName()));
177+
if (logger.isTraceEnabled()) {
178+
logger.trace(LogMessage.format("Authenticating request with %s (%d/%d)",
179+
provider.getClass().getSimpleName(), ++currentPosition, size));
180+
}
176181
try {
177182
result = provider.authenticate(authentication);
178183
if (result != null) {
@@ -220,6 +225,7 @@ public Authentication authenticate(Authentication authentication) throws Authent
220225
if (parentResult == null) {
221226
this.eventPublisher.publishAuthenticationSuccess(result);
222227
}
228+
223229
return result;
224230
}
225231

‎core/src/main/java/org/springframework/security/authentication/dao/AbstractUserDetailsAuthenticationProvider.java

+10-5
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ public Authentication authenticate(Authentication authentication) throws Authent
133133
user = retrieveUser(username, (UsernamePasswordAuthenticationToken) authentication);
134134
}
135135
catch (UsernameNotFoundException ex) {
136-
this.logger.debug("User '" + username + "' not found");
136+
this.logger.debug("Failed to find user '" + username + "'");
137137
if (!this.hideUserNotFoundExceptions) {
138138
throw ex;
139139
}
@@ -196,6 +196,7 @@ protected Authentication createSuccessAuthentication(Object principal, Authentic
196196
UsernamePasswordAuthenticationToken result = new UsernamePasswordAuthenticationToken(principal,
197197
authentication.getCredentials(), this.authoritiesMapper.mapAuthorities(user.getAuthorities()));
198198
result.setDetails(authentication.getDetails());
199+
this.logger.debug("Authenticated user");
199200
return result;
200201
}
201202

@@ -318,17 +319,20 @@ private class DefaultPreAuthenticationChecks implements UserDetailsChecker {
318319
@Override
319320
public void check(UserDetails user) {
320321
if (!user.isAccountNonLocked()) {
321-
AbstractUserDetailsAuthenticationProvider.this.logger.debug("User account is locked");
322+
AbstractUserDetailsAuthenticationProvider.this.logger
323+
.debug("Failed to authenticate since user account is locked");
322324
throw new LockedException(AbstractUserDetailsAuthenticationProvider.this.messages
323325
.getMessage("AbstractUserDetailsAuthenticationProvider.locked", "User account is locked"));
324326
}
325327
if (!user.isEnabled()) {
326-
AbstractUserDetailsAuthenticationProvider.this.logger.debug("User account is disabled");
328+
AbstractUserDetailsAuthenticationProvider.this.logger
329+
.debug("Failed to authenticate since user account is disabled");
327330
throw new DisabledException(AbstractUserDetailsAuthenticationProvider.this.messages
328331
.getMessage("AbstractUserDetailsAuthenticationProvider.disabled", "User is disabled"));
329332
}
330333
if (!user.isAccountNonExpired()) {
331-
AbstractUserDetailsAuthenticationProvider.this.logger.debug("User account is expired");
334+
AbstractUserDetailsAuthenticationProvider.this.logger
335+
.debug("Failed to authenticate since user account has expired");
332336
throw new AccountExpiredException(AbstractUserDetailsAuthenticationProvider.this.messages
333337
.getMessage("AbstractUserDetailsAuthenticationProvider.expired", "User account has expired"));
334338
}
@@ -341,7 +345,8 @@ private class DefaultPostAuthenticationChecks implements UserDetailsChecker {
341345
@Override
342346
public void check(UserDetails user) {
343347
if (!user.isCredentialsNonExpired()) {
344-
AbstractUserDetailsAuthenticationProvider.this.logger.debug("User account credentials have expired");
348+
AbstractUserDetailsAuthenticationProvider.this.logger
349+
.debug("Failed to authenticate since user account credentials have expired");
345350
throw new CredentialsExpiredException(AbstractUserDetailsAuthenticationProvider.this.messages
346351
.getMessage("AbstractUserDetailsAuthenticationProvider.credentialsExpired",
347352
"User credentials have expired"));

‎core/src/main/java/org/springframework/security/authentication/dao/DaoAuthenticationProvider.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,13 @@ public DaoAuthenticationProvider() {
6868
protected void additionalAuthenticationChecks(UserDetails userDetails,
6969
UsernamePasswordAuthenticationToken authentication) throws AuthenticationException {
7070
if (authentication.getCredentials() == null) {
71-
this.logger.debug("Authentication failed: no credentials provided");
71+
this.logger.debug("Failed to authenticate since no credentials provided");
7272
throw new BadCredentialsException(this.messages
7373
.getMessage("AbstractUserDetailsAuthenticationProvider.badCredentials", "Bad credentials"));
7474
}
7575
String presentedPassword = authentication.getCredentials().toString();
7676
if (!this.passwordEncoder.matches(presentedPassword, userDetails.getPassword())) {
77-
this.logger.debug("Authentication failed: password does not match stored value");
77+
this.logger.debug("Failed to authenticate since password does not match stored value");
7878
throw new BadCredentialsException(this.messages
7979
.getMessage("AbstractUserDetailsAuthenticationProvider.badCredentials", "Bad credentials"));
8080
}

‎core/src/main/java/org/springframework/security/core/context/SecurityContextImpl.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,14 @@ public void setAuthentication(Authentication authentication) {
7373
@Override
7474
public String toString() {
7575
StringBuilder sb = new StringBuilder();
76-
sb.append(super.toString());
76+
sb.append(getClass().getSimpleName()).append(" [");
7777
if (this.authentication == null) {
78-
sb.append(": Null authentication");
78+
sb.append("Null authentication");
7979
}
8080
else {
81-
sb.append(": Authentication: ").append(this.authentication);
81+
sb.append("Authentication=").append(this.authentication);
8282
}
83+
sb.append("]");
8384
return sb.toString();
8485
}
8586

‎core/src/main/java/org/springframework/security/core/userdetails/User.java

+8-21
Original file line numberDiff line numberDiff line change
@@ -196,27 +196,14 @@ public int hashCode() {
196196
@Override
197197
public String toString() {
198198
StringBuilder sb = new StringBuilder();
199-
sb.append(super.toString()).append(": ");
200-
sb.append("Username: ").append(this.username).append("; ");
201-
sb.append("Password: [PROTECTED]; ");
202-
sb.append("Enabled: ").append(this.enabled).append("; ");
203-
sb.append("AccountNonExpired: ").append(this.accountNonExpired).append("; ");
204-
sb.append("credentialsNonExpired: ").append(this.credentialsNonExpired).append("; ");
205-
sb.append("AccountNonLocked: ").append(this.accountNonLocked).append("; ");
206-
if (!this.authorities.isEmpty()) {
207-
sb.append("Granted Authorities: ");
208-
boolean first = true;
209-
for (GrantedAuthority auth : this.authorities) {
210-
if (!first) {
211-
sb.append(",");
212-
}
213-
first = false;
214-
sb.append(auth);
215-
}
216-
}
217-
else {
218-
sb.append("Not granted any authorities");
219-
}
199+
sb.append(getClass().getName()).append(" [");
200+
sb.append("Username=").append(this.username).append(", ");
201+
sb.append("Password=[PROTECTED], ");
202+
sb.append("Enabled=").append(this.enabled).append(", ");
203+
sb.append("AccountNonExpired=").append(this.accountNonExpired).append(", ");
204+
sb.append("credentialsNonExpired=").append(this.credentialsNonExpired).append(", ");
205+
sb.append("AccountNonLocked=").append(this.accountNonLocked).append(", ");
206+
sb.append("Granted Authorities=").append(this.authorities).append("]");
220207
return sb.toString();
221208
}
222209

0 commit comments

Comments
 (0)