Skip to content

Commit

Permalink
Add Logging to Resource Server
Browse files Browse the repository at this point in the history
Closes gh-9000
  • Loading branch information
jzheaux committed Sep 8, 2020
1 parent 593a556 commit bf067d6
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@
import com.nimbusds.jwt.proc.ConfigurableJWTProcessor;
import com.nimbusds.jwt.proc.DefaultJWTProcessor;
import com.nimbusds.jwt.proc.JWTProcessor;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.cache.Cache;
import org.springframework.core.convert.converter.Converter;
Expand Down Expand Up @@ -80,6 +82,8 @@
*/
public final class NimbusJwtDecoder implements JwtDecoder {

private final Log logger = LogFactory.getLog(getClass());

private static final String DECODING_ERROR_MESSAGE_TEMPLATE = "An error occurred while attempting to decode the Jwt: %s";

private final JWTProcessor<SecurityContext> jwtProcessor;
Expand Down Expand Up @@ -126,6 +130,7 @@ public void setClaimSetConverter(Converter<Map<String, Object>, Map<String, Obje
public Jwt decode(String token) throws JwtException {
JWT jwt = parse(token);
if (jwt instanceof PlainJWT) {
this.logger.trace("Failed to decode unsigned token");
throw new BadJwtException("Unsupported algorithm of " + jwt.getHeader().getAlgorithm());
}
Jwt createdJwt = createJwt(token, jwt);
Expand All @@ -137,6 +142,7 @@ private JWT parse(String token) {
return JWTParser.parse(token);
}
catch (Exception ex) {
this.logger.trace("Failed to parse token", ex);
throw new BadJwtException(String.format(DECODING_ERROR_MESSAGE_TEMPLATE, ex.getMessage()), ex);
}
}
Expand All @@ -155,15 +161,18 @@ private Jwt createJwt(String token, JWT parsedJwt) {
// @formatter:on
}
catch (RemoteKeySourceException ex) {
this.logger.trace("Failed to retrieve JWK set", ex);
if (ex.getCause() instanceof ParseException) {
throw new JwtException(String.format(DECODING_ERROR_MESSAGE_TEMPLATE, "Malformed Jwk set"));
}
throw new JwtException(String.format(DECODING_ERROR_MESSAGE_TEMPLATE, ex.getMessage()), ex);
}
catch (JOSEException ex) {
this.logger.trace("Failed to process JWT", ex);
throw new JwtException(String.format(DECODING_ERROR_MESSAGE_TEMPLATE, ex.getMessage()), ex);
}
catch (Exception ex) {
this.logger.trace("Failed to process JWT", ex);
if (ex.getCause() instanceof ParseException) {
throw new BadJwtException(String.format(DECODING_ERROR_MESSAGE_TEMPLATE, "Malformed payload"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@

import java.util.Collection;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.core.convert.converter.Converter;
import org.springframework.security.authentication.AbstractAuthenticationToken;
import org.springframework.security.authentication.AuthenticationProvider;
Expand Down Expand Up @@ -60,6 +63,8 @@
*/
public final class JwtAuthenticationProvider implements AuthenticationProvider {

private final Log logger = LogFactory.getLog(getClass());

private final JwtDecoder jwtDecoder;

private Converter<Jwt, ? extends AbstractAuthenticationToken> jwtAuthenticationConverter = new JwtAuthenticationConverter();
Expand All @@ -83,6 +88,7 @@ public Authentication authenticate(Authentication authentication) throws Authent
Jwt jwt = getJwt(bearer);
AbstractAuthenticationToken token = this.jwtAuthenticationConverter.convert(jwt);
token.setDetails(bearer.getDetails());
this.logger.debug("Authenticated token");
return token;
}

Expand All @@ -91,6 +97,7 @@ private Jwt getJwt(BearerTokenAuthenticationToken bearer) {
return this.jwtDecoder.decode(bearer.getToken());
}
catch (BadJwtException failed) {
this.logger.debug("Failed to authenticate since the JWT was invalid");
throw new InvalidBearerTokenException(failed.getMessage(), failed);
}
catch (JwtException failed) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@
import java.util.Collection;
import java.util.Collections;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.core.convert.converter.Converter;
import org.springframework.core.log.LogMessage;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.oauth2.jwt.Jwt;
Expand All @@ -37,6 +41,8 @@
*/
public final class JwtGrantedAuthoritiesConverter implements Converter<Jwt, Collection<GrantedAuthority>> {

private final Log logger = LogFactory.getLog(getClass());

private static final String DEFAULT_AUTHORITY_PREFIX = "SCOPE_";

private static final Collection<String> WELL_KNOWN_AUTHORITIES_CLAIM_NAMES = Arrays.asList("scope", "scp");
Expand Down Expand Up @@ -98,8 +104,12 @@ private String getAuthoritiesClaimName(Jwt jwt) {
private Collection<String> getAuthorities(Jwt jwt) {
String claimName = getAuthoritiesClaimName(jwt);
if (claimName == null) {
this.logger.trace("Returning no authorities since could not find any claims that might contain scopes");
return Collections.emptyList();
}
if (this.logger.isTraceEnabled()) {
this.logger.trace(LogMessage.format("Looking for scopes in claim %s", claimName));
}
Object authorities = jwt.getClaim(claimName);
if (authorities instanceof String) {
if (StringUtils.hasText((String) authorities)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@
import javax.servlet.http.HttpServletRequest;

import com.nimbusds.jwt.JWTParser;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.core.convert.converter.Converter;
import org.springframework.core.log.LogMessage;
import org.springframework.lang.NonNull;
import org.springframework.security.authentication.AuthenticationManager;
import org.springframework.security.authentication.AuthenticationManagerResolver;
Expand Down Expand Up @@ -151,6 +154,8 @@ public String convert(@NonNull HttpServletRequest request) {
private static class TrustedIssuerJwtAuthenticationManagerResolver
implements AuthenticationManagerResolver<String> {

private final Log logger = LogFactory.getLog(getClass());

private final Map<String, AuthenticationManager> authenticationManagers = new ConcurrentHashMap<>();

private final Predicate<String> trustedIssuer;
Expand All @@ -162,10 +167,17 @@ private static class TrustedIssuerJwtAuthenticationManagerResolver
@Override
public AuthenticationManager resolve(String issuer) {
if (this.trustedIssuer.test(issuer)) {
return this.authenticationManagers.computeIfAbsent(issuer, (k) -> {
JwtDecoder jwtDecoder = JwtDecoders.fromIssuerLocation(issuer);
return new JwtAuthenticationProvider(jwtDecoder)::authenticate;
});
AuthenticationManager authenticationManager = this.authenticationManagers.computeIfAbsent(issuer,
(k) -> {
this.logger.debug("Constructing AuthenticationManager");
JwtDecoder jwtDecoder = JwtDecoders.fromIssuerLocation(issuer);
return new JwtAuthenticationProvider(jwtDecoder)::authenticate;
});
this.logger.debug(LogMessage.format("Resolved AuthenticationManager for issuer '%s'", issuer));
return authenticationManager;
}
else {
this.logger.debug("Did not resolve AuthenticationManager since issuer is not trusted");
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
import java.time.Instant;
import java.util.Collection;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.security.authentication.AbstractAuthenticationToken;
import org.springframework.security.authentication.AuthenticationProvider;
import org.springframework.security.authentication.AuthenticationServiceException;
Expand Down Expand Up @@ -61,6 +64,8 @@
*/
public final class OpaqueTokenAuthenticationProvider implements AuthenticationProvider {

private final Log logger = LogFactory.getLog(getClass());

private OpaqueTokenIntrospector introspector;

/**
Expand Down Expand Up @@ -89,6 +94,7 @@ public Authentication authenticate(Authentication authentication) throws Authent
OAuth2AuthenticatedPrincipal principal = getOAuth2AuthenticatedPrincipal(bearer);
AbstractAuthenticationToken result = convert(principal, bearer.getToken());
result.setDetails(bearer.getDetails());
this.logger.debug("Authenticated token");
return result;
}

Expand All @@ -97,6 +103,7 @@ private OAuth2AuthenticatedPrincipal getOAuth2AuthenticatedPrincipal(BearerToken
return this.introspector.introspect(bearer.getToken());
}
catch (BadOpaqueTokenException failed) {
this.logger.debug("Failed to authenticate since token was invalid");
throw new InvalidBearerTokenException(failed.getMessage());
}
catch (OAuth2IntrospectionException failed) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import com.nimbusds.oauth2.sdk.TokenIntrospectionSuccessResponse;
import com.nimbusds.oauth2.sdk.http.HTTPResponse;
import com.nimbusds.oauth2.sdk.id.Audience;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.core.convert.converter.Converter;
import org.springframework.http.HttpHeaders;
Expand Down Expand Up @@ -58,6 +60,8 @@
*/
public class NimbusOpaqueTokenIntrospector implements OpaqueTokenIntrospector {

private final Log logger = LogFactory.getLog(getClass());

private Converter<String, RequestEntity<?>> requestEntityConverter;

private RestOperations restOperations;
Expand Down Expand Up @@ -128,6 +132,7 @@ public OAuth2AuthenticatedPrincipal introspect(String token) {
// relying solely on the authorization server to validate this token (not checking
// 'exp', for example)
if (!introspectionSuccessResponse.isActive()) {
this.logger.trace("Did not validate token since it is inactive");
throw new BadOpaqueTokenException("Provided token isn't active");
}
return convertClaimsSet(introspectionSuccessResponse);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.springframework.core.log.LogMessage;
import org.springframework.security.authentication.AuthenticationDetailsSource;
import org.springframework.security.authentication.AuthenticationManager;
import org.springframework.security.authentication.AuthenticationManagerResolver;
Expand Down Expand Up @@ -105,10 +106,12 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
token = this.bearerTokenResolver.resolve(request);
}
catch (OAuth2AuthenticationException invalid) {
this.logger.trace("Sending to authentication entry point since failed to resolve bearer token", invalid);
this.authenticationEntryPoint.commence(request, response, invalid);
return;
}
if (token == null) {
this.logger.trace("Did not process request since did not find bearer token");
filterChain.doFilter(request, response);
return;
}
Expand All @@ -120,11 +123,14 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
SecurityContext context = SecurityContextHolder.createEmptyContext();
context.setAuthentication(authenticationResult);
SecurityContextHolder.setContext(context);
if (this.logger.isDebugEnabled()) {
this.logger.debug(LogMessage.format("Set SecurityContextHolder to %s", authenticationResult));
}
filterChain.doFilter(request, response);
}
catch (AuthenticationException failed) {
SecurityContextHolder.clearContext();
this.logger.debug("Authentication request for failed!", failed);
this.logger.trace("Failed to process authentication request", failed);
this.authenticationFailureHandler.onAuthenticationFailure(request, response, failed);
}
}
Expand Down

0 comments on commit bf067d6

Please sign in to comment.