From ba468c7e6e0714d469791c0a5f9715d483cbf843 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Tue, 12 Oct 2021 12:56:36 -0600 Subject: [PATCH] Restructure SwitchUserFilter Logs Issue gh-6311 --- .../switchuser/SwitchUserFilter.java | 15 +++++++++------ .../authentication/SwitchUserWebFilter.java | 17 +++++++++++------ 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/authentication/switchuser/SwitchUserFilter.java b/web/src/main/java/org/springframework/security/web/authentication/switchuser/SwitchUserFilter.java index 11e1ed94ba..d68d520757 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/switchuser/SwitchUserFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/switchuser/SwitchUserFilter.java @@ -175,6 +175,7 @@ public class SwitchUserFilter extends GenericFilterBean implements ApplicationEv Authentication targetUser = attemptSwitchUser(request); // update the current context to the new target user SecurityContextHolder.getContext().setAuthentication(targetUser); + this.logger.debug(LogMessage.format("Set SecurityContextHolder to %s", targetUser)); // redirect to target url this.successHandler.onAuthenticationSuccess(request, response, targetUser); } @@ -189,10 +190,13 @@ public class SwitchUserFilter extends GenericFilterBean implements ApplicationEv Authentication originalUser = attemptExitUser(request); // update the current context back to the original user SecurityContextHolder.getContext().setAuthentication(originalUser); + this.logger.debug(LogMessage.format("Set SecurityContextHolder to %s", originalUser)); // redirect to target url this.successHandler.onAuthenticationSuccess(request, response, originalUser); return; } + this.logger.trace(LogMessage.format("Did not attempt to switch user since request did not match [%s] or [%s]", + this.switchUserMatcher, this.exitUserMatcher)); chain.doFilter(request, response); } @@ -211,12 +215,11 @@ public class SwitchUserFilter extends GenericFilterBean implements ApplicationEv UsernamePasswordAuthenticationToken targetUserRequest; String username = request.getParameter(this.usernameParameter); username = (username != null) ? username : ""; - this.logger.debug(LogMessage.format("Attempt to switch to user [%s]", username)); + this.logger.debug(LogMessage.format("Attempting to switch to user [%s]", username)); UserDetails targetUser = this.userDetailsService.loadUserByUsername(username); this.userDetailsChecker.check(targetUser); // OK, create the switch user token targetUserRequest = createSwitchUserToken(request, targetUser); - this.logger.debug(LogMessage.format("Switch User Token [%s]", targetUserRequest)); // publish event if (this.eventPublisher != null) { this.eventPublisher.publishEvent(new AuthenticationSwitchUserEvent( @@ -245,9 +248,9 @@ public class SwitchUserFilter extends GenericFilterBean implements ApplicationEv // if so, get the original source user so we can switch back Authentication original = getSourceAuthentication(current); if (original == null) { - this.logger.debug("Could not find original user Authentication object!"); - throw new AuthenticationCredentialsNotFoundException(this.messages.getMessage( - "SwitchUserFilter.noOriginalAuthentication", "Could not find original Authentication object")); + this.logger.debug("Failed to find original user"); + throw new AuthenticationCredentialsNotFoundException(this.messages + .getMessage("SwitchUserFilter.noOriginalAuthentication", "Failed to find original user")); } // get the source user details UserDetails originalUser = null; @@ -322,7 +325,7 @@ public class SwitchUserFilter extends GenericFilterBean implements ApplicationEv // check for switch user type of authority if (auth instanceof SwitchUserGrantedAuthority) { original = ((SwitchUserGrantedAuthority) auth).getSource(); - this.logger.debug("Found original switch user granted authority [" + original + "]"); + this.logger.debug(LogMessage.format("Found original switch user granted authority [%s]", original)); } } return original; diff --git a/web/src/main/java/org/springframework/security/web/server/authentication/SwitchUserWebFilter.java b/web/src/main/java/org/springframework/security/web/server/authentication/SwitchUserWebFilter.java index 410da7cb6c..fc63312d3e 100644 --- a/web/src/main/java/org/springframework/security/web/server/authentication/SwitchUserWebFilter.java +++ b/web/src/main/java/org/springframework/security/web/server/authentication/SwitchUserWebFilter.java @@ -158,8 +158,12 @@ public class SwitchUserWebFilter implements WebFilter { public Mono filter(ServerWebExchange exchange, WebFilterChain chain) { final WebFilterExchange webFilterExchange = new WebFilterExchange(exchange, chain); return switchUser(webFilterExchange).switchIfEmpty(Mono.defer(() -> exitSwitchUser(webFilterExchange))) - .switchIfEmpty(Mono.defer(() -> chain.filter(exchange).then(Mono.empty()))) - .flatMap((authentication) -> onAuthenticationSuccess(authentication, webFilterExchange)) + .switchIfEmpty(Mono.defer(() -> { + this.logger.trace( + LogMessage.format("Did not attempt to switch user since request did not match [%s] or [%s]", + this.switchUserMatcher, this.exitUserMatcher)); + return chain.filter(exchange).then(Mono.empty()); + })).flatMap((authentication) -> onAuthenticationSuccess(authentication, webFilterExchange)) .onErrorResume(SwitchUserAuthenticationException.class, (exception) -> Mono.empty()); } @@ -211,7 +215,7 @@ public class SwitchUserWebFilter implements WebFilter { @NonNull private Mono attemptSwitchUser(Authentication currentAuthentication, String userName) { Assert.notNull(userName, "The userName can not be null."); - this.logger.debug(LogMessage.format("Attempt to switch to user [%s]", userName)); + this.logger.debug(LogMessage.format("Attempting to switch to user [%s]", userName)); return this.userDetailsService.findByUsername(userName) .switchIfEmpty(Mono.error(this::noTargetAuthenticationException)) .doOnNext(this.userDetailsChecker::check) @@ -222,7 +226,7 @@ public class SwitchUserWebFilter implements WebFilter { private Authentication attemptExitUser(Authentication currentAuthentication) { Optional sourceAuthentication = extractSourceAuthentication(currentAuthentication); if (!sourceAuthentication.isPresent()) { - this.logger.debug("Could not find original user Authentication object!"); + this.logger.debug("Failed to find original user"); throw noOriginalAuthenticationException(); } return sourceAuthentication.get(); @@ -232,13 +236,14 @@ public class SwitchUserWebFilter implements WebFilter { ServerWebExchange exchange = webFilterExchange.getExchange(); SecurityContextImpl securityContext = new SecurityContextImpl(authentication); return this.securityContextRepository.save(exchange, securityContext) + .doOnSuccess((v) -> this.logger.debug(LogMessage.format("Switched user to %s", authentication))) .then(this.successHandler.onAuthenticationSuccess(webFilterExchange, authentication)) .subscriberContext(ReactiveSecurityContextHolder.withSecurityContext(Mono.just(securityContext))); } private Mono onAuthenticationFailure(AuthenticationException exception, WebFilterExchange webFilterExchange) { return Mono.justOrEmpty(this.failureHandler).switchIfEmpty(Mono.defer(() -> { - this.logger.error("Switch User failed", exception); + this.logger.debug("Failed to switch user", exception); return Mono.error(exception); })).flatMap((failureHandler) -> failureHandler.onAuthenticationFailure(webFilterExchange, exception)); } @@ -247,7 +252,7 @@ public class SwitchUserWebFilter implements WebFilter { Optional sourceAuthentication = extractSourceAuthentication(currentAuthentication); if (sourceAuthentication.isPresent()) { // SEC-1763. Check first if we are already switched. - this.logger.info( + this.logger.debug( LogMessage.format("Found original switch user granted authority [%s]", sourceAuthentication.get())); currentAuthentication = sourceAuthentication.get(); }