Skip to content

Commit

Permalink
Remove permissions from jti
Browse files Browse the repository at this point in the history
Signed-off-by: Derek Ho <[email protected]>
  • Loading branch information
derek-ho committed Dec 30, 2024
1 parent 665b9e9 commit e39df0d
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 89 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public String createApiToken(
) {
apiTokenIndexHandler.createApiTokenIndexIfAbsent();
// TODO: Add validation on whether user is creating a token with a subset of their permissions
ExpiringBearerAuthToken token = securityTokenManager.issueApiToken(name, expiration, clusterPermissions, indexPermissions);
ExpiringBearerAuthToken token = securityTokenManager.issueApiToken(name, expiration);
ApiToken apiToken = new ApiToken(
name,
securityTokenManager.encryptToken(token.getCompleteToken()),
Expand Down
34 changes: 2 additions & 32 deletions src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

package org.opensearch.security.authtoken.jwt;

import java.io.IOException;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.text.ParseException;
Expand All @@ -27,10 +26,6 @@
import org.opensearch.OpenSearchException;
import org.opensearch.common.collect.Tuple;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.xcontent.XContentFactory;
import org.opensearch.core.xcontent.ToXContent;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.security.action.apitokens.ApiToken;

import com.nimbusds.jose.JOSEException;
import com.nimbusds.jose.JWSAlgorithm;
Expand Down Expand Up @@ -157,14 +152,8 @@ public ExpiringBearerAuthToken createJwt(
}

@SuppressWarnings("removal")
public ExpiringBearerAuthToken createJwt(
final String issuer,
final String subject,
final String audience,
final long expiration,
final List<String> clusterPermissions,
final List<ApiToken.IndexPermission> indexPermissions
) throws JOSEException, ParseException, IOException {
public ExpiringBearerAuthToken createJwt(final String issuer, final String subject, final String audience, final long expiration)
throws JOSEException, ParseException {
final long currentTimeMs = timeProvider.getAsLong();
final Date now = new Date(currentTimeMs);

Expand All @@ -178,25 +167,6 @@ public ExpiringBearerAuthToken createJwt(
final Date expiryTime = new Date(expiration);
claimsBuilder.expirationTime(expiryTime);

if (clusterPermissions != null) {
final String listOfClusterPermissions = String.join(",", clusterPermissions);
claimsBuilder.claim("cp", encryptString(listOfClusterPermissions));
}

if (indexPermissions != null) {
XContentBuilder builder = XContentFactory.jsonBuilder();
builder.startArray();
for (ApiToken.IndexPermission permission : indexPermissions) {
// Add each permission to the array
permission.toXContent(builder, ToXContent.EMPTY_PARAMS);
}
builder.endArray();

// Encrypt the entire JSON array
String jsonArray = builder.toString();
claimsBuilder.claim("ip", encryptString(jsonArray));
}

final JWSHeader header = new JWSHeader.Builder(JWSAlgorithm.parse(signingKey.getAlgorithm().getName())).build();

final SignedJWT signedJwt = AccessController.doPrivileged(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,9 @@ private AuthCredentials extractCredentials0(final SecurityRequest request, final
log.error("Cannot authenticate user with JWT because of ", e);
return null;
} catch (Exception e) {
log.error("Invalid or expired JWT token.", e);
if (log.isDebugEnabled()) {
log.debug("Invalid or expired JWT token.", e);
}
}

// Return null for the authentication failure
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
package org.opensearch.security.identity;

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.Set;

Expand All @@ -28,7 +27,6 @@
import org.opensearch.identity.tokens.AuthToken;
import org.opensearch.identity.tokens.OnBehalfOfClaims;
import org.opensearch.identity.tokens.TokenManager;
import org.opensearch.security.action.apitokens.ApiToken;
import org.opensearch.security.authtoken.jwt.ExpiringBearerAuthToken;
import org.opensearch.security.authtoken.jwt.JwtVendor;
import org.opensearch.security.securityconf.ConfigModel;
Expand Down Expand Up @@ -141,16 +139,11 @@ public ExpiringBearerAuthToken issueOnBehalfOfToken(final Subject subject, final
}
}

public ExpiringBearerAuthToken issueApiToken(
final String name,
final Long expiration,
final List<String> clusterPermissions,
final List<ApiToken.IndexPermission> indexPermissions
) {
public ExpiringBearerAuthToken issueApiToken(final String name, final Long expiration) {
final User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);

try {
return apiTokenJwtVendor.createJwt(cs.getClusterName().value(), name, name, expiration, clusterPermissions, indexPermissions);
return apiTokenJwtVendor.createJwt(cs.getClusterName().value(), name, name, expiration);
} catch (final Exception ex) {
logger.error("Error creating Api Token for " + user.getName(), ex);
throw new OpenSearchSecurityException("Unable to generate Api Token");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

package org.opensearch.security.action.apitokens;

import java.util.List;

import org.apache.logging.log4j.Logger;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -81,7 +83,7 @@ public void testExtractCredentialsPassWhenJtiInCache() {
String encryptedTestJti =
"k3JQNRXR57Y4V4W1LNkpEP+QzcDra5+/fFfQrr0Rncr/RhYAnYEDgN9RKcJ2Wmjf63NCQa9+HjeFcn0H2wKhMm7pl9bd3ya9FO+LUD7t9Sih4DOjUt0t7ee4ROC0eRK5glMsKsKQVkuY+YKa6A6dT8bMqmt7kIrer7w8TRENr9J8x41TGb/cDDWDvJLME7QkFzJjMxYDgKNiEevMbOpC8yjIZdK08jPe3Thq+xm+JYruoYeyI5g8QjkJA9ZOs1f6eXTAvPxhseuPqgIKykRE25fuWjl5n9tJ9W+jpl+iET7zzOLXSPEU5UepL/COkVd6xW63Ay72oMOewqveDXzyR8S8LAfgRuKgYZWms7yT37XcGg0c6Y7M62KVPo+1XQ+F+K5bgddkd8G+I9KHf561jIMzBcIodgGRj659954W16D1C92+PF/YWPQoTv2hVK4f60H82ga1YSiz3r9UrFV8d7gLJwtyJT9HNPuXO2VZ7xPhre+n1Wv7No0kH2S/r3nqKK6Bk/kn1ZbAmjLxuw13c95lIir6avlKE7XX4PiQDfcGeAyeXOw/36kLW8wH7kjXWdBspld1AiI4fCOaszNXF+7gcuTxIhECl+mEyrJbMI88EWllq+LbydiOrVLFXXRMiCbvj+VTYjzimgJPp+Vuvg==";
ApiTokenIndexListenerCache cache = ApiTokenIndexListenerCache.getInstance();
cache.getJtis().put(encryptedTestJti, null);
cache.getJtis().put(encryptedTestJti, new Permissions(List.of(), List.of()));
assertTrue(cache.getJtis().containsKey(encryptedTestJti));

SecurityRequest request = mock(SecurityRequest.class);
Expand All @@ -100,7 +102,7 @@ public void testExtractCredentialsFailWhenTokenIsExpired() {
String encryptedTestJti =
"k3JQNRXR57Y4V4W1LNkpEP+QzcDra5+/fFfQrr0Rncr/RhYAnYEDgN9RKcJ2Wmjf63NCQa9+HjeFcn0H2wKhMm7pl9bd3ya9FO+LUD7t9ShsbOyBUkpFSVuQwrXLatY+glMsKsKQVkuY+YKa6A6dT8bMqmt7kIrer7w8TRENr9J8x41TGb/cDDWDvJLME7QkFzJjMxYDgKNiEevMbOpC8yjIZdK08jPe3Thq+xm+JYruoYeyI5g8QjkJA9ZOs1f6eXTAvPxhseuPqgIKykRE25fuWjl5n9tJ9W+jpl+iET7zzOLXSPEU5UepL/COkVd6xW63Ay72oMOewqveDXzyR8S8LAfgRuKgYZWms7yT37XcGg0c6Y7M62KVPo+1XQ+Fu193YtvS4vqt9G8jHiq51VCRxNHYVlAsratxzvECD8AKBilR9/7dUKyOQDBIzPG4ws+kgI680SgdMgGuLANQPGzal9US8GsWzTbQWCgtObaSVKB02U4gh16wvy3XrXtPz2Z0ZAxoU2Z8opX8hcvB5MG5UUEf+tpgTtVPcbuJyCL42yD3FIc3v/LCYlG/hFvflXBx5c1r+4Tij8Qc/NkYb7/03xiJsVH6eduSqR9M0QBpLm7xg2TgqVMvC/+n96x2V3lS4via4lAK6xuYeRY0ng==";
ApiTokenIndexListenerCache cache = ApiTokenIndexListenerCache.getInstance();
cache.getJtis().put(encryptedTestJti, null);
cache.getJtis().put(encryptedTestJti, new Permissions(List.of(), List.of()));
assertTrue(cache.getJtis().containsKey(encryptedTestJti));

SecurityRequest request = mock(SecurityRequest.class);
Expand All @@ -121,7 +123,7 @@ public void testExtractCredentialsFailWhenIssuerDoesNotMatch() {
String encryptedTestJti =
"k3JQNRXR57Y4V4W1LNkpEP+QzcDra5+/fFfQrr0Rncr/RhYAnYEDgN9RKcJ2Wmjf63NCQa9+HjeFcn0H2wKhMm7pl9bd3ya9FO+LUD7t9Sih4DOjUt0t7ee4ROC0eRK5glMsKsKQVkuY+YKa6A6dT8bMqmt7kIrer7w8TRENr9J8x41TGb/cDDWDvJLME7QkFzJjMxYDgKNiEevMbOpC8yjIZdK08jPe3Thq+xm+JYruoYeyI5g8QjkJA9ZOs1f6eXTAvPxhseuPqgIKykRE25fuWjl5n9tJ9W+jpl+iET7zzOLXSPEU5UepL/COkVd6xW63Ay72oMOewqveDXzyR8S8LAfgRuKgYZWms7yT37XcGg0c6Y7M62KVPo+1XQ+F+K5bgddkd8G+I9KHf561jIMzBcIodgGRj659954W16D1C92+PF/YWPQoTv2hVK4f60H82ga1YSiz3r9UrFV8d7gLJwtyJT9HNPuXO2VZ7xPhre+n1Wv7No0kH2S/r3nqKK6Bk/kn1ZbAmjLxuw13c95lIir6avlKE7XX4PiQDfcGeAyeXOw/36kLW8wH7kjXWdBspld1AiI4fCOaszNXF+7gcuTxIhECl+mEyrJbMI88EWllq+LbydiOrVLFXXRMiCbvj+VTYjzimgJPp+Vuvg==";
ApiTokenIndexListenerCache cache = ApiTokenIndexListenerCache.getInstance();
cache.getJtis().put(encryptedTestJti, null);
cache.getJtis().put(encryptedTestJti, new Permissions(List.of(), List.of()));
assertTrue(cache.getJtis().containsKey(encryptedTestJti));

SecurityRequest request = mock(SecurityRequest.class);
Expand Down Expand Up @@ -150,7 +152,7 @@ public void testExtractCredentialsFailWhenAccessingRestrictedEndpoint() {
String encryptedTestJti =
"k3JQNRXR57Y4V4W1LNkpEP+QzcDra5+/fFfQrr0Rncr/RhYAnYEDgN9RKcJ2Wmjf63NCQa9+HjeFcn0H2wKhMm7pl9bd3ya9FO+LUD7t9Sih4DOjUt0t7ee4ROC0eRK5glMsKsKQVkuY+YKa6A6dT8bMqmt7kIrer7w8TRENr9J8x41TGb/cDDWDvJLME7QkFzJjMxYDgKNiEevMbOpC8yjIZdK08jPe3Thq+xm+JYruoYeyI5g8QjkJA9ZOs1f6eXTAvPxhseuPqgIKykRE25fuWjl5n9tJ9W+jpl+iET7zzOLXSPEU5UepL/COkVd6xW63Ay72oMOewqveDXzyR8S8LAfgRuKgYZWms7yT37XcGg0c6Y7M62KVPo+1XQ+F+K5bgddkd8G+I9KHf561jIMzBcIodgGRj659954W16D1C92+PF/YWPQoTv2hVK4f60H82ga1YSiz3r9UrFV8d7gLJwtyJT9HNPuXO2VZ7xPhre+n1Wv7No0kH2S/r3nqKK6Bk/kn1ZbAmjLxuw13c95lIir6avlKE7XX4PiQDfcGeAyeXOw/36kLW8wH7kjXWdBspld1AiI4fCOaszNXF+7gcuTxIhECl+mEyrJbMI88EWllq+LbydiOrVLFXXRMiCbvj+VTYjzimgJPp+Vuvg==";
ApiTokenIndexListenerCache cache = ApiTokenIndexListenerCache.getInstance();
cache.getJtis().put(encryptedTestJti, null);
cache.getJtis().put(encryptedTestJti, new Permissions(List.of(), List.of()));
assertTrue(cache.getJtis().containsKey(encryptedTestJti));

SecurityRequest request = mock(SecurityRequest.class);
Expand All @@ -169,7 +171,7 @@ public void testAuthenticatorNotEnabled() {
String encryptedTestJti =
"k3JQNRXR57Y4V4W1LNkpEP+QzcDra5+/fFfQrr0Rncr/RhYAnYEDgN9RKcJ2Wmjf63NCQa9+HjeFcn0H2wKhMm7pl9bd3ya9FO+LUD7t9Sih4DOjUt0t7ee4ROC0eRK5glMsKsKQVkuY+YKa6A6dT8bMqmt7kIrer7w8TRENr9J8x41TGb/cDDWDvJLME7QkFzJjMxYDgKNiEevMbOpC8yjIZdK08jPe3Thq+xm+JYruoYeyI5g8QjkJA9ZOs1f6eXTAvPxhseuPqgIKykRE25fuWjl5n9tJ9W+jpl+iET7zzOLXSPEU5UepL/COkVd6xW63Ay72oMOewqveDXzyR8S8LAfgRuKgYZWms7yT37XcGg0c6Y7M62KVPo+1XQ+F+K5bgddkd8G+I9KHf561jIMzBcIodgGRj659954W16D1C92+PF/YWPQoTv2hVK4f60H82ga1YSiz3r9UrFV8d7gLJwtyJT9HNPuXO2VZ7xPhre+n1Wv7No0kH2S/r3nqKK6Bk/kn1ZbAmjLxuw13c95lIir6avlKE7XX4PiQDfcGeAyeXOw/36kLW8wH7kjXWdBspld1AiI4fCOaszNXF+7gcuTxIhECl+mEyrJbMI88EWllq+LbydiOrVLFXXRMiCbvj+VTYjzimgJPp+Vuvg==";
ApiTokenIndexListenerCache cache = ApiTokenIndexListenerCache.getInstance();
cache.getJtis().put(encryptedTestJti, null);
cache.getJtis().put(encryptedTestJti, new Permissions(List.of(), List.of()));
assertTrue(cache.getJtis().containsKey(encryptedTestJti));

SecurityRequest request = mock(SecurityRequest.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,13 @@ public void testCreateApiToken() {
String encryptedToken = "encrypted-token";
ExpiringBearerAuthToken bearerToken = mock(ExpiringBearerAuthToken.class);
when(bearerToken.getCompleteToken()).thenReturn(completeToken);
when(securityTokenManager.issueApiToken(any(), any(), any(), any())).thenReturn(bearerToken);
when(securityTokenManager.issueApiToken(any(), any())).thenReturn(bearerToken);
when(securityTokenManager.encryptToken(completeToken)).thenReturn(encryptedToken);

String result = repository.createApiToken(tokenName, clusterPermissions, indexPermissions, expiration);

verify(apiTokenIndexHandler).createApiTokenIndexIfAbsent();
verify(securityTokenManager).issueApiToken(any(), any(), any(), any());
verify(securityTokenManager).issueApiToken(any(), any());
verify(securityTokenManager).encryptToken(completeToken);
verify(apiTokenIndexHandler).indexTokenMetadata(
argThat(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,7 @@
import org.opensearch.common.collect.Tuple;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.xcontent.XContentFactory;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.core.xcontent.DeprecationHandler;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.core.xcontent.ToXContent;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.security.action.apitokens.ApiToken;
import org.opensearch.security.support.ConfigConstants;

Expand Down Expand Up @@ -297,14 +293,7 @@ public void testCreateJwtForApiTokenSuccess() throws Exception {
String claimsEncryptionKey = "1234567890123456";
Settings settings = Settings.builder().put("signing_key", signingKeyB64Encoded).put("encryption_key", claimsEncryptionKey).build();
final JwtVendor jwtVendor = new JwtVendor(settings, Optional.of(currentTime));
final ExpiringBearerAuthToken authToken = jwtVendor.createJwt(
issuer,
subject,
audience,
Long.MAX_VALUE,
clusterPermissions,
indexPermissions
);
final ExpiringBearerAuthToken authToken = jwtVendor.createJwt(issuer, subject, audience, Long.MAX_VALUE);

SignedJWT signedJWT = SignedJWT.parse(authToken.getCompleteToken());

Expand All @@ -314,29 +303,6 @@ public void testCreateJwtForApiTokenSuccess() throws Exception {
assertThat(signedJWT.getJWTClaimsSet().getClaims().get("iat"), is(notNullValue()));
// Allow for millisecond to second conversion flexibility
assertThat(((Date) signedJWT.getJWTClaimsSet().getClaims().get("exp")).getTime() / 1000, equalTo(Long.MAX_VALUE / 1000));

EncryptionDecryptionUtil encryptionUtil = new EncryptionDecryptionUtil(claimsEncryptionKey);
assertThat(
encryptionUtil.decrypt(signedJWT.getJWTClaimsSet().getClaims().get("cp").toString()),
equalTo(expectedClusterPermissions)
);

assertThat(encryptionUtil.decrypt(signedJWT.getJWTClaimsSet().getClaims().get("ip").toString()), equalTo(expectedIndexPermissions));

XContentParser parser = XContentType.JSON.xContent()
.createParser(
NamedXContentRegistry.EMPTY,
DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
encryptionUtil.decrypt(signedJWT.getJWTClaimsSet().getClaims().get("ip").toString())
);
// Parse first item of the list
parser.nextToken();
parser.nextToken();
ApiToken.IndexPermission indexPermission1 = ApiToken.IndexPermission.fromXContent(parser);

// Index permission deserialization works as expected
assertThat(indexPermission1.getIndexPatterns(), equalTo(indexPermission.getIndexPatterns()));
assertThat(indexPermission1.getAllowedActions(), equalTo(indexPermission.getAllowedActions()));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,8 @@ public void issueApiToken_success() throws Exception {
createMockJwtVendorInTokenManager();

final ExpiringBearerAuthToken authToken = mock(ExpiringBearerAuthToken.class);
when(jwtVendor.createJwt(anyString(), anyString(), anyString(), anyLong(), any(), any())).thenReturn(authToken);
final AuthToken returnedToken = tokenManager.issueApiToken("elmo", Long.MAX_VALUE, List.of("*"), List.of());
when(jwtVendor.createJwt(anyString(), anyString(), anyString(), anyLong())).thenReturn(authToken);
final AuthToken returnedToken = tokenManager.issueApiToken("elmo", Long.MAX_VALUE);

assertThat(returnedToken, equalTo(authToken));

Expand All @@ -282,8 +282,8 @@ public void encryptCallsJwtEncrypt() throws Exception {
createMockJwtVendorInTokenManager();

final ExpiringBearerAuthToken authToken = mock(ExpiringBearerAuthToken.class);
when(jwtVendor.createJwt(anyString(), anyString(), anyString(), anyLong(), any(), any())).thenReturn(authToken);
final AuthToken returnedToken = tokenManager.issueApiToken("elmo", Long.MAX_VALUE, List.of("*"), List.of());
when(jwtVendor.createJwt(anyString(), anyString(), anyString(), anyLong())).thenReturn(authToken);
final AuthToken returnedToken = tokenManager.issueApiToken("elmo", Long.MAX_VALUE);

assertThat(returnedToken, equalTo(authToken));

Expand Down

0 comments on commit e39df0d

Please sign in to comment.