Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed Authorization header from logs [#57] #58

Merged
merged 2 commits into from
Oct 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion jwt-opa/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ ext {
}

group 'com.alertavert'
version '0.10.0'
version '0.11.0'

// OpenJDK 17 LTS is the only Java version supported
sourceCompatibility = JavaVersion.VERSION_17
Expand Down
1 change: 0 additions & 1 deletion jwt-opa/src/main/java/com/alertavert/opa/Constants.java
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ public Collection<? extends GrantedAuthority> getAuthorities() {
@Override public boolean isCredentialsNonExpired() {return false;}
@Override public boolean isEnabled() {return false;}
};
public static final int MAX_TOKEN_LEN_LOG = 6;
public static final ObjectMapper MAPPER = new ObjectMapper();
public static final String PEM_EXT = ".pem";
public static final String PUB_EXT = ".pub";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import com.auth0.jwt.exceptions.JWTVerificationException;
import com.auth0.jwt.interfaces.DecodedJWT;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.security.authentication.BadCredentialsException;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.GrantedAuthority;
Expand All @@ -31,8 +30,6 @@

import java.util.List;

import static com.alertavert.opa.Constants.MAX_TOKEN_LEN_LOG;

/**
* <h2>ApiTokenAuthenticationFactory</h2>
*
Expand Down Expand Up @@ -62,7 +59,7 @@ public ApiTokenAuthenticationFactory(JwtTokenProvider provider) {
* grant with the {@link JwtTokenProvider#ROLES} carried by the JWT.
*/
public Mono<Authentication> createAuthentication(String token) {
log.debug("Authenticating token {}...", token.substring(0, Math.min(MAX_TOKEN_LEN_LOG, token.length())));
log.debug("Authenticating token {}", JwtTokenProvider.maskToken(token));
try {
DecodedJWT jwt = provider.decode(token);
List<? extends GrantedAuthority> authorities = AuthorityUtils.createAuthorityList(
Expand Down
35 changes: 29 additions & 6 deletions jwt-opa/src/main/java/com/alertavert/opa/jwt/JwtTokenProvider.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import com.auth0.jwt.interfaces.DecodedJWT;
import com.auth0.jwt.interfaces.JWTVerifier;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.lang.Nullable;
import org.springframework.security.authentication.BadCredentialsException;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
Expand All @@ -51,14 +50,38 @@
@Component
@Slf4j
public class JwtTokenProvider {

public static final String ROLES = "roles";
private static final String MASK = "****";
private static final int MASKED_TOKEN_LEN = 12;

private final Algorithm hmac;
private final TokensProperties tokensProperties;

@Autowired
Algorithm hmac;
public JwtTokenProvider(Algorithm hmac, TokensProperties tokensProperties) {
this.hmac = hmac;
this.tokensProperties = tokensProperties;
}

@Autowired
TokensProperties tokensProperties;
/**
* This method takes an API Token and masks it by replacing the middle part with {@link #MASK},
* and the first and last characters of the token, so that the total length is
* {@link #MASKED_TOKEN_LEN}.
*
* @param token the API Token to mask
* @return a masked version of the token
*/
public static String maskToken(String token) {
if (token == null) {
return "";
}
var totLen = Math.min(token.length(), MASKED_TOKEN_LEN);
if (totLen <= MASK.length()) {
return MASK;
}
int prefixLen = Math.max((totLen - MASK.length()) / 2, 1);
return token.substring(0, prefixLen) + MASK
+ token.substring(token.length() - prefixLen);
}

public JWTVerifier verifier() {
return JWT.require(hmac)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public class OpaReactiveAuthorizationManager

@PostConstruct
private void info() {
log.info("Configured Headers, headers = {}", requiredHeaders);
log.info("Configured headers = {}", requiredHeaders);
}

/**
Expand Down Expand Up @@ -142,37 +142,25 @@ private TokenBasedAuthorizationRequest makeRequestBody(
) {
Map<String, String> authnHeaders = new HashMap<>();
HttpHeaders requestHeaders = request.getHeaders();
log.debug("Adding headers, request = {}, required = {}", requestHeaders,
log.debug("Adding headers, request = {}, required = {}", requestHeaders.keySet(),
requiredHeaders);
if (requestHeaders != null) {
requiredHeaders.forEach(key -> {
var value = requestHeaders.getFirst(key);
if (value != null) {
authnHeaders.put(key, value);
}
});
}
requiredHeaders.forEach(key -> {
var value = requestHeaders.getFirst(key);
if (value != null) {
authnHeaders.put(key, value);
}
});

String token = Objects.requireNonNull(credentials).toString();
return TokenBasedAuthorizationRequest.builder()
.input(new TokenBasedAuthorizationRequest.AuthRequestBody(token,
new TokenBasedAuthorizationRequest.Resource(
request.getMethodValue(),
request.getMethod().name(),
request.getPath().toString(),
authnHeaders
)
)
)
.build();
}

private WebClientResponseException unauthorized() {
return WebClientResponseException.create(
HttpStatus.UNAUTHORIZED.value(),
HttpStatus.UNAUTHORIZED.getReasonPhrase(),
null,
USER_NOT_AUTHORIZED.getBytes(StandardCharsets.UTF_8),
StandardCharsets.UTF_8
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,24 @@

package com.alertavert.opa.security;

import com.alertavert.opa.jwt.JwtTokenProvider;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.FormatFeature;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.PrettyPrinter;
import com.fasterxml.jackson.core.util.DefaultPrettyPrinter;
import lombok.Builder;
import lombok.Value;
import lombok.extern.jackson.Jacksonized;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpHeaders;

import java.util.Formattable;
import java.util.Formatter;
import java.util.Map;
import java.util.stream.Collectors;

import static com.alertavert.opa.Constants.MAPPER;
import static com.alertavert.opa.Constants.MAX_TOKEN_LEN_LOG;


/**
Expand All @@ -40,17 +48,17 @@
* "input"} object, we use this class to simplify the construction of the JSON body.
*
// {{ formatter:off }}
<pre>
{
"input": {
"api_token": ".... API Token Base-64 encoded ...",
"resource": {
"method": "POST",
"path": "/path/to/resource"
}
}
}
</pre>
<pre>
{
"input": {
"api_token": ".... API Token Base-64 encoded ...",
"resource": {
"method": "POST",
"path": "/path/to/resource"
}
}
}
</pre>
// {{ formatter:on }}
*
* <p>When serializing to String (e.g., in debug logs output) the API Token (JWT) is obfuscated
Expand All @@ -62,30 +70,43 @@
@Value
@Builder
@Jacksonized
@Slf4j
public class TokenBasedAuthorizationRequest {

public record Resource(String method, String path, Map<String, ?> headers) {
}

public record AuthRequestBody(@JsonProperty("api_token") String token, Resource resource) {
public record AuthRequestBody(@JsonProperty("api_token") String token,
Resource resource) implements Formattable {

private static final PrettyPrinter PRETTY_PRINTER = new DefaultPrettyPrinter()
.withoutSpacesInObjectEntries();

@Override
public void formatTo(Formatter formatter, int flags, int width, int precision) {
// Creates a new Map with all the headers, excluding the Authorization one.
Map<String, ?> maskedHeaders = resource.headers().entrySet().stream()
.filter(e -> !e.getKey().equalsIgnoreCase(HttpHeaders.AUTHORIZATION))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
try {
AuthRequestBody copy = new AuthRequestBody(JwtTokenProvider.maskToken(token),
new Resource(resource.method(), resource.path(), maskedHeaders));
formatter.format("%s", MAPPER.writer().with(PRETTY_PRINTER)
.writeValueAsString(copy));
} catch (JsonProcessingException e) {
formatter.format("invalid JSON: %s", e.getMessage());
}
}
}

AuthRequestBody input;

@Override
public String toString() {
try {
String token = "";
if (input.token.length() > 2 * MAX_TOKEN_LEN_LOG) {
token = input.token.substring(0, MAX_TOKEN_LEN_LOG) + "****" +
input.token.substring(input.token.length() - MAX_TOKEN_LEN_LOG);
}
TokenBasedAuthorizationRequest copy = TokenBasedAuthorizationRequest.builder()
.input(new AuthRequestBody(token, input.resource))
.build();
return MAPPER.writerWithDefaultPrettyPrinter().writeValueAsString(copy);
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
}
return String.format("""
{
"input": %s
}
""", input);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ void check() {
}

@Test
public void checkUnauthorizedFails() {
void checkUnauthorizedFails() {
Authentication auth = factory.createAuthentication(
provider.createToken("alice", Lists.list("USER"))
).block();
Expand All @@ -121,7 +121,7 @@ public void checkUnauthorizedFails() {
}

@Test
public void checkUnauthenticatedFails() {
void checkUnauthenticatedFails() {
Authentication auth = new UsernamePasswordAuthenticationToken("bob", "pass");

// As this endpoint is not mapped in `routes` (application-test.yaml) it expects by default
Expand All @@ -142,7 +142,7 @@ private AuthorizationContext getAuthorizationContextWithHeaders(
ServerHttpRequest request = mock(ServerHttpRequest.class);
RequestPath requestPath = mock(RequestPath.class);

when(request.getMethodValue()).thenReturn(method.name());
when(request.getMethod()).thenReturn(method);
when(request.getPath()).thenReturn(requestPath);
when(requestPath.toString()).thenReturn(path);

Expand All @@ -159,7 +159,7 @@ private AuthorizationContext getAuthorizationContextWithHeaders(
}

@Test
public void authenticatedEndpointBypassesOpa() {
void authenticatedEndpointBypassesOpa() {
AuthorizationContext context = getAuthorizationContext(HttpMethod.GET, "/testauth");
assertThat(opaReactiveAuthorizationManager.check(
factory.createAuthentication(
Expand All @@ -170,7 +170,7 @@ public void authenticatedEndpointBypassesOpa() {
}

@Test
public void authenticatedEndpointMatches() {
void authenticatedEndpointMatches() {
// In the test configuration (application-test.yaml) we have configured the following
// path matchers: ["/match/*/this", "/match/any/**"].
// Here we test that an authenticated user gains access to them without needing authorization.
Expand Down Expand Up @@ -202,7 +202,7 @@ public void authenticatedEndpointMatches() {
}

@Test
public void testHeaders() {
void testHeaders() {
AuthorizationContext context = getAuthorizationContextWithHeaders(HttpMethod.GET, "/whatever",
Map.of("x-test-header", "test-value", HttpHeaders.USER_AGENT, "TestAgent"));
assertThat(opaReactiveAuthorizationManager.check(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
import static com.alertavert.opa.Constants.MAPPER;
import static com.jayway.jsonpath.matchers.JsonPathMatchers.hasJsonPath;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.jupiter.api.Assertions.assertFalse;

class TokenBasedAuthorizationRequestTest {

Expand All @@ -48,15 +50,18 @@ void serialize() throws Exception {
@Test
void obfuscatesJwt() {
TokenBasedAuthorizationRequest request = TokenBasedAuthorizationRequest.builder()
.input(new AuthRequestBody("tokenAAjwtDEF123456.anothertoken.yetanothertoken",
.input(new AuthRequestBody("AAjwtDEF123456.ZZZfere43535.yYYYY98764awarkfajser",
new TokenBasedAuthorizationRequest.Resource("POST", "/foo/bar", Map.of()))
)
.build();
String json = request.toString();

assertThat(json, hasJsonPath("$.input"));
assertThat(json, hasJsonPath("$.input.api_token", equalTo("tokenA****rtoken")));
assertThat(json, hasJsonPath("$.input.resource.method", equalTo("POST")));
assertThat(json, hasJsonPath("$.input.resource.path", equalTo("/foo/bar")));
assertThat(request.toString(), containsString("AAjw****jser"));
assertFalse(request.toString().matches(".*AAjwtDEF123456\\.ZZZfere43535\\"
+ ".yYYYY98764awarkfajser.*"));
assertThat(request.toString(), containsString("""
"method":"POST",
"""));
assertThat(request.toString(), containsString("""
"path":"/foo/bar",
"""));
}
}
4 changes: 2 additions & 2 deletions webapp-example/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ dependencies {
// as to emulate an actual project using jwt-opa externally.
// Uncomment the following line (and comment out the one below) to use the local version
// while developing.
// implementation project (':jwt-opa')
implementation "com.alertavert:jwt-opa:${jwtOpaVersion}"
implementation project (':jwt-opa')
// implementation "com.alertavert:jwt-opa:${jwtOpaVersion}"

// For the @PostConstruct annotation
implementation 'javax.annotation:javax.annotation-api:1.3.2'
Expand Down
Loading
Loading