Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* information: "Portions copyright [year] [name of copyright owner]".
*
* Copyright 2019 Open Identity Platform Community.
* Portions copyright 2025 3A Systems LLC.
* Portions copyright 2025-2026 3A Systems LLC.
*/

package org.openidentityplatform.openam.cassandra;
Expand Down Expand Up @@ -68,6 +68,20 @@
public class TokenStorageAdapter implements org.forgerock.openam.sm.datalayer.api.TokenStorageAdapter {
final static Logger logger = LoggerFactory.getLogger(TokenStorageAdapter.class);

/**
* Renders a token id for log output. The id is a session id or an OAuth2
* token, so only a short prefix is kept: the raw value must never reach the logs.
*/
static String maskTokenId(String tokenId) {
if (tokenId == null) {
return "null";
}
if (tokenId.length() <= 8) {
return "***";
}
return tokenId.substring(0, 4) + "***";
}

private final DataLayerConfiguration cfg;
static ConnectionFactory<CqlSession> connectionFactory;

Expand Down Expand Up @@ -111,7 +125,7 @@ public Token update(Token token, boolean ifExists) throws DataLayerException {
try {
value = token.getAttribute(field);
}catch (Throwable e) {
logger.warn("create {} for {} {}",e.toString(),field,token);
logger.warn("update: unable to read {} of {} token {}: {}",field,token.getType(),maskTokenId(token.getTokenId()),e.toString());
throw e;
}
if (value!=null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* The contents of this file are subject to the terms of the Common Development and
* Distribution License (the License). You may not use this file except in compliance with the
* License.
*
* You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the
* specific language governing permission and limitations under the License.
*
* When distributing Covered Software, include this CDDL Header Notice in each file and include
* the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL
* Header, with the fields enclosed by brackets [] replaced by your own identifying
* information: "Portions copyright [year] [name of copyright owner]".
*
* Copyright 2026 3A Systems, LLC.
*/
package org.openidentityplatform.openam.cassandra;

import static org.junit.Assert.assertEquals;

import org.junit.Test;

public class TokenStorageAdapterTest {

/**
* A CTS token id is a session id or an OAuth2 token: it must never be written
* to the log in full, only a short prefix that identifies the record.
*/
@Test
public void maskTokenIdKeepsOnlyAShortPrefix() {
assertEquals("AQIC***", TokenStorageAdapter.maskTokenId("AQIC5wM2LY4SfczntBcXfFoFJwA6zAV2i4fnU8Sd7ao"));
assertEquals("***", TokenStorageAdapter.maskTokenId("short"));
assertEquals("***", TokenStorageAdapter.maskTokenId(""));
assertEquals("null", TokenStorageAdapter.maskTokenId(null));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* information: "Portions copyright [year] [name of copyright owner]".
*
* Copyright 2019 Open Identity Platform Community.
* Portions copyright 2026 3A Systems LLC.
*/

package org.openidentityplatform.openam.cassandra;
Expand Down Expand Up @@ -434,15 +435,16 @@
}catch (IdRepoException e) {
throw e;
}catch(Throwable e){
logger.error("setAttributes {} {} {} {}",type,name,attributes_in,isAdd,e.getMessage());
// Log attribute names only: the values may include userPassword.
logger.error("setAttributes {} {} {} {}: {}",type,name,attributes_in.keySet(),isAdd,e.getMessage());
Comment thread
vharseko marked this conversation as resolved.
Dismissed
Comment thread
vharseko marked this conversation as resolved.
Dismissed
throw new IdRepoException(e.getMessage());
}
}

@Override
public void setBinaryAttributes(SSOToken token, IdType type, String name,Map<String, byte[][]> attributes, boolean isAdd) throws IdRepoException, SSOException {
//validate(type, IdOperation.EDIT);
logger.warn("unsupported setBinaryAttributes {} {} {} {}",type,name,attributes,isAdd);
logger.warn("unsupported setBinaryAttributes {} {} {} {}",type,name,attributes.keySet(),isAdd);
Comment thread
vharseko marked this conversation as resolved.
Dismissed
Comment thread
vharseko marked this conversation as resolved.
Dismissed
throw new IdRepoUnsupportedOpException("unsupported setBinaryAttributes");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,21 @@
this.tokenCache = tokenCache;
}

/**
* Renders a session id or access token for log output. Only a short prefix is
* kept: a token written to a log file is enough to hijack the session it
* represents, so the raw value must never reach the logs.
*/
static String maskToken(String token) {
if (token == null) {
return "null";
}
if (token.length() <= 8) {
return "***";
}
return token.substring(0, 4) + "***";
}

@Override
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception {
if(openAMConfig.useOAuthForAuthentication()) {
Expand Down Expand Up @@ -125,7 +140,7 @@
return true;
}
log.info("preHandleUsernamePassword: token {} is about to expire in {} s (attempt {}/{})",
token, seconds, attempt + 1, MAX_TOKEN_RESOLUTION_ATTEMPTS);
maskToken(token), seconds, attempt + 1, MAX_TOKEN_RESOLUTION_ATTEMPTS);
tokenCache.invalidate(LOGIN_PASSWORD_TOKEN_KEY);
token = getUserNamePasswordToken();
tokenCache.put(LOGIN_PASSWORD_TOKEN_KEY, token);
Expand Down Expand Up @@ -165,7 +180,7 @@
request.setAttribute("tokenId", token);
return true;
}
log.info("preHandleOAuth: token {} is about to expire in {} s", token, seconds);
log.info("preHandleOAuth: token {} is about to expire in {} s", maskToken(token), seconds);
tokenCache.invalidate(accessToken);
token = getTokenIdFromAccessToken(accessToken);
tokenCache.put(accessToken, token);
Expand All @@ -190,7 +205,7 @@
if(response.containsKey("name")) {
return true;
} else {
log.warn("got invalid response: {} for access token: {}", response, accessToken);
log.warn("got invalid response: {} for access token: {}", response, maskToken(accessToken));
Comment thread
vharseko marked this conversation as resolved.
Dismissed
return false;
}
} catch (Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package org.openidentityplatform.openam.mcp.server.security;

import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.read.ListAppender;
import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.Caffeine;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -24,17 +26,23 @@
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.openidentityplatform.openam.mcp.server.config.OpenAMConfig;
import org.slf4j.LoggerFactory;
import org.springframework.core.ParameterizedTypeReference;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.web.client.RestClient;

import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.atMost;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
Expand Down Expand Up @@ -227,4 +235,95 @@ public void preHandleUsernamePassword_logsInAndCachesToken_whenCacheIsEmpty() th
assertThat(req.getAttribute("tokenId")).isEqualTo(newToken);
assertThat(tokenCache.getIfPresent("login-password-token")).isEqualTo(newToken);
}

@Test
void maskToken_keepsOnlyAShortPrefix() {
assertThat(AuthInterceptor.maskToken("AQIC5wM2LY4SfczntBcXfFoFJwA6zAV2i4fnU8Sd7ao")).isEqualTo("AQIC***");
assertThat(AuthInterceptor.maskToken("short")).isEqualTo("***");
assertThat(AuthInterceptor.maskToken("")).isEqualTo("***");
assertThat(AuthInterceptor.maskToken(null)).isEqualTo("null");
}

/**
* A session id or an access token in the log file lets anyone who can read
* the logs hijack that session, so the interceptor must never log them raw.
*/
@Test
void preHandleUsernamePassword_doesNotLogRawToken_whenRefreshing() throws Exception {
String expiredToken = "AQIC5wM2LY4Sfczn-expired-session-token";
String freshToken = "AQIC5wM2LY4Sfczn-fresh-session-token";
tokenCache.put("login-password-token", expiredToken);

AuthInterceptor spy = spy(interceptor);
doReturn(1L).when(spy).tokenValidSeconds(expiredToken);
doReturn(freshToken).when(spy).getUserNamePasswordToken();
doReturn(300L).when(spy).tokenValidSeconds(freshToken);

List<String> messages = captureLogs(() -> spy.preHandleUsernamePassword(new MockHttpServletRequest()));

assertThat(messages).anyMatch(m -> m.contains("about to expire"));
assertThat(messages).noneMatch(m -> m.contains(expiredToken));
assertThat(messages).noneMatch(m -> m.contains(freshToken));
}

@Test
void preHandleOAuth_doesNotLogRawSessionToken_whenRefreshing() {
String accessToken = "f3c1a9e0-access-token-value";
String expiredToken = "AQIC5wM2LY4Sfczn-expired-session-token";
tokenCache.put(accessToken, expiredToken);

MockHttpServletRequest req = new MockHttpServletRequest();
req.addHeader("Authorization", "Bearer " + accessToken);

AuthInterceptor spy = spy(interceptor);
doReturn(true).when(spy).accessTokenValid(accessToken);
doReturn(1L).when(spy).tokenValidSeconds(expiredToken);

List<String> messages = captureLogs(() -> {
try {
spy.preHandleOAuth(req, new MockHttpServletResponse());
} catch (Exception ignored) {
// The mocked RestClient cannot mint a new token; the log line
// under test is written before that call.
}
});

assertThat(messages).anyMatch(m -> m.contains("about to expire"));
assertThat(messages).noneMatch(m -> m.contains(expiredToken));
}

@Test
void accessTokenValid_doesNotLogRawAccessToken_onInvalidResponse() {
String accessToken = "f3c1a9e0-access-token-value";

// userinfo answers without a "name": the token must be reported as invalid
// without echoing it into the log.
@SuppressWarnings("rawtypes")
RestClient.RequestHeadersUriSpec uriSpec = mock(RestClient.RequestHeadersUriSpec.class);
RestClient.ResponseSpec responseSpec = mock(RestClient.ResponseSpec.class);
doReturn(uriSpec).when(restClient).get();
doReturn(uriSpec).when(uriSpec).uri(anyString());
doReturn(uriSpec).when(uriSpec).header(anyString(), any(String[].class));
doReturn(responseSpec).when(uriSpec).retrieve();
doReturn(Map.of("error", "invalid_token")).when(responseSpec).body(any(ParameterizedTypeReference.class));

List<String> messages = captureLogs(() -> assertThat(interceptor.accessTokenValid(accessToken)).isFalse());

assertThat(messages).anyMatch(m -> m.contains("got invalid response"));
assertThat(messages).noneMatch(m -> m.contains(accessToken));
}

private static List<String> captureLogs(Runnable action) {
ch.qos.logback.classic.Logger logger =
(ch.qos.logback.classic.Logger) LoggerFactory.getLogger(AuthInterceptor.class);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
logger.addAppender(appender);
try {
action.run();
} finally {
logger.detachAppender(appender);
}
return appender.list.stream().map(ILoggingEvent::getFormattedMessage).collect(Collectors.toList());
}
}
Loading