Skip to content

Commit

Permalink
Move secret validation logic and authN-loadPrincipal into `PolarisSec…
Browse files Browse the repository at this point in the history 
…retsManager`

The logic _how_ a principal and/or principal secret's are persisted should be transparent to the calling code. Relying on the persistence internals for principals and secrets management makes it impossible to factor out secrets management / make principal management possible.

This change moves the secret validation and retrieval of a principal by client-ID behind an implementation of `PolarisSecretsManager`.
  • Loading branch information
@snazy
snazy committed Dec 9, 2024
1 parent edd09f0 commit 226558f
Show file tree
Hide file tree
Showing 10 changed files with 143 additions and 46 deletions.
43 changes: 43 additions & 0 deletions polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@
import jakarta.annotation.Nonnull;
import jakarta.annotation.Nullable;
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.entity.PolarisBaseEntity;
import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
import org.apache.polaris.core.persistence.BaseResult;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager.EntityResult;

/** Manages secrets for Polaris principals. */
public interface PolarisSecretsManager {
Expand All @@ -39,6 +41,17 @@ public interface PolarisSecretsManager {
PrincipalSecretsResult loadPrincipalSecrets(
@Nonnull PolarisCallContext callCtx, @Nonnull String clientId);

@Nonnull
SecretValidationResult validateSecret(
@Nonnull PolarisCallContext callCtx, @Nonnull String clientId, @Nonnull String clientSecret);

@Nonnull
EntityResult loadPrincipal(
@Nonnull PolarisCallContext callCtx,
@Nullable String roleName,
@Nullable String clientId,
@Nullable Long principalId);

/**
* Rotate secrets
*
Expand Down Expand Up @@ -100,4 +113,34 @@ public PolarisPrincipalSecrets getPrincipalSecrets() {
return principalSecrets;
}
}

/** the result of load/rotate principal secrets */
class SecretValidationResult extends BaseResult {

private final PolarisBaseEntity principal;

public SecretValidationResult(
@Nonnull BaseResult.ReturnStatus errorCode, @Nullable String extraInformation) {
super(errorCode, extraInformation);
this.principal = null;
}

public SecretValidationResult(@Nonnull PolarisBaseEntity principal) {
super(BaseResult.ReturnStatus.SUCCESS);
this.principal = principal;
}

@JsonCreator
private SecretValidationResult(
@JsonProperty("returnStatus") @Nonnull BaseResult.ReturnStatus returnStatus,
@JsonProperty("extraInformation") @Nullable String extraInformation,
@JsonProperty("principalSecrets") @Nonnull PolarisBaseEntity principal) {
super(returnStatus, extraInformation);
this.principal = principal;
}

public PolarisBaseEntity getPrincipal() {
return principal;
}
}
}
2 changes: 2 additions & 0 deletions polaris-core/src/main/java/org/apache/polaris/core/persistence/BaseResult.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ public enum ReturnStatus {

// error caught while sub-scoping credentials. Error message will be returned
SUBSCOPE_CREDS_ERROR(13),

SECRET_VALIDATION_FAILED(14),
;

// code for the enum
Expand Down
2 changes: 1 addition & 1 deletion polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisEntityManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public Resolver prepareResolver(
callContext.getPolarisCallContext(),
metaStoreManager,
authenticatedPrincipal.getPrincipalEntity().getId(),
null, /* callerPrincipalName */
authenticatedPrincipal.getPrincipalEntity().getName(),
authenticatedPrincipal.getActivatedPrincipalRoleNames().isEmpty()
? null
: authenticatedPrincipal.getActivatedPrincipalRoleNames(),
Expand Down
58 changes: 58 additions & 0 deletions ...s-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
*/
package org.apache.polaris.core.persistence;

import static com.google.common.base.Preconditions.checkArgument;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.JsonMappingException;
Expand Down Expand Up @@ -55,6 +57,7 @@
import org.apache.polaris.core.storage.PolarisStorageActions;
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
import org.apache.polaris.core.storage.PolarisStorageIntegration;
import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -1006,6 +1009,61 @@ public Map<String, String> deserializeProperties(PolarisCallContext callCtx, Str
: new PrincipalSecretsResult(secrets);
}

@Override
public @NotNull SecretValidationResult validateSecret(
@NotNull PolarisCallContext callCtx, @NotNull String clientId, @NotNull String clientSecret) {
PrincipalSecretsResult principalSecrets = loadPrincipalSecrets(callCtx, clientId);
if (!principalSecrets.isSuccess()
|| !principalSecrets.getPrincipalSecrets().matchesSecret(clientSecret)) {
return new SecretValidationResult(BaseResult.ReturnStatus.SECRET_VALIDATION_FAILED, "");
}
PolarisMetaStoreManager.EntityResult result =
loadEntity(callCtx, 0L, principalSecrets.getPrincipalSecrets().getPrincipalId());
if (!result.isSuccess() || result.getEntity().getType() != PolarisEntityType.PRINCIPAL) {
return new SecretValidationResult(BaseResult.ReturnStatus.SECRET_VALIDATION_FAILED, "");
}
return new SecretValidationResult(result.getEntity());
}

@Override
public @NotNull EntityResult loadPrincipal(
@NotNull PolarisCallContext callCtx,
@Nullable String roleName,
@Nullable String clientId,
@Nullable Long principalId) {
checkArgument(principalId != null || clientId != null || roleName != null);
if (principalId != null && principalId > 0) {
EntityResult result = loadEntity(callCtx, 0L, principalId);
if (result.isSuccess() && result.getEntity().getType() == PolarisEntityType.PRINCIPAL) {
return result;
}
}
if (roleName != null) {
EntityResult result =
readEntityByName(
callCtx,
null,
PolarisEntityType.PRINCIPAL,
PolarisEntitySubType.NULL_SUBTYPE,
roleName);
if (result.isSuccess()) {
return result;
}
}
if (clientId != null) {
PrincipalSecretsResult principalSecrets = loadPrincipalSecrets(callCtx, clientId);
if (principalSecrets.isSuccess()
&& principalSecrets.getPrincipalSecrets().getPrincipalId() > 0) {
EntityResult result =
loadEntity(callCtx, 0L, principalSecrets.getPrincipalSecrets().getPrincipalId());
if (result.isSuccess() && result.getEntity().getType() == PolarisEntityType.PRINCIPAL) {
return result;
}
}
}
return new EntityResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, "");
}

/** See {@link #} */
private @Nullable PolarisPrincipalSecrets rotatePrincipalSecrets(
@Nonnull PolarisCallContext callCtx,
Expand Down
18 changes: 18 additions & 0 deletions ...c/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.entity.PolarisPrivilege;
import org.apache.polaris.core.storage.PolarisStorageActions;
import org.jetbrains.annotations.NotNull;

/**
* Wraps an existing impl of PolarisMetaStoreManager and delegates expected "read" operations
Expand Down Expand Up @@ -129,6 +130,23 @@ public PrincipalSecretsResult loadPrincipalSecrets(
return null;
}

@Override
public @NotNull SecretValidationResult validateSecret(
@NotNull PolarisCallContext callCtx, @NotNull String clientId, @NotNull String clientSecret) {
callCtx.getDiagServices().fail("illegal_method_in_transaction_workspace", "validateSecret");
return null;
}

@Override
public @NotNull EntityResult loadPrincipal(
@NotNull PolarisCallContext callCtx,
@Nullable String roleName,
@Nullable String clientId,
@Nullable Long principalId) {
callCtx.getDiagServices().fail("illegal_method_in_transaction_workspace", "validateSecret");
return null;
}

@Override
public PrincipalSecretsResult rotatePrincipalSecrets(
@Nonnull PolarisCallContext callCtx,
Expand Down
19 changes: 6 additions & 13 deletions polaris-service/src/main/java/org/apache/polaris/service/auth/BasePolarisAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@
import org.apache.polaris.core.context.CallContext;
import org.apache.polaris.core.context.RealmContext;
import org.apache.polaris.core.entity.PolarisEntity;
import org.apache.polaris.core.entity.PolarisEntitySubType;
import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.entity.PrincipalEntity;
import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
Expand Down Expand Up @@ -70,17 +68,12 @@ protected Optional<AuthenticatedPolarisPrincipal> getPrincipal(DecodedToken toke
PolarisEntity principal;
try {
principal =
tokenInfo.getPrincipalId() > 0
? PolarisEntity.of(
metaStoreManager.loadEntity(
getCurrentPolarisContext(), 0L, tokenInfo.getPrincipalId()))
: PolarisEntity.of(
metaStoreManager.readEntityByName(
getCurrentPolarisContext(),
null,
PolarisEntityType.PRINCIPAL,
PolarisEntitySubType.NULL_SUBTYPE,
tokenInfo.getSub()));
PolarisEntity.of(
metaStoreManager.loadPrincipal(
getCurrentPolarisContext(),
tokenInfo.getSub(),
tokenInfo.getClientId(),
tokenInfo.getPrincipalId()));
} catch (Exception e) {
LOGGER
.atError()
Expand Down
20 changes: 5 additions & 15 deletions polaris-service/src/main/java/org/apache/polaris/service/auth/TokenBroker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@
import jakarta.annotation.Nonnull;
import java.util.Optional;
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.auth.PolarisSecretsManager.PrincipalSecretsResult;
import org.apache.polaris.core.auth.PolarisSecretsManager;
import org.apache.polaris.core.context.CallContext;
import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.entity.PrincipalEntity;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
import org.apache.polaris.service.types.TokenType;
Expand All @@ -47,20 +46,11 @@ TokenResponse generateFromToken(
PolarisMetaStoreManager metaStoreManager, String clientId, String clientSecret) {
// Validate the principal is present and secrets match
PolarisCallContext polarisCallContext = CallContext.getCurrentContext().getPolarisCallContext();
PrincipalSecretsResult principalSecrets =
metaStoreManager.loadPrincipalSecrets(polarisCallContext, clientId);
if (!principalSecrets.isSuccess()) {
PolarisSecretsManager.SecretValidationResult result =
metaStoreManager.validateSecret(polarisCallContext, clientId, clientSecret);
if (!result.isSuccess()) {
return Optional.empty();
}
if (!principalSecrets.getPrincipalSecrets().matchesSecret(clientSecret)) {
return Optional.empty();
}
PolarisMetaStoreManager.EntityResult result =
metaStoreManager.loadEntity(
polarisCallContext, 0L, principalSecrets.getPrincipalSecrets().getPrincipalId());
if (!result.isSuccess() || result.getEntity().getType() != PolarisEntityType.PRINCIPAL) {
return Optional.empty();
}
return Optional.of(PrincipalEntity.of(result.getEntity()));
return Optional.of(PrincipalEntity.of(result.getPrincipal()));
}
}
5 changes: 4 additions & 1 deletion ...ice/src/test/java/org/apache/polaris/service/admin/PolarisServiceImplIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2355,7 +2355,10 @@ public void testTokenInvalidSignature() {
@Test
public void testTokenInvalidPrincipalId() {
String newToken =
defaultJwt().withClaim(CLAIM_KEY_PRINCIPAL_ID, 0).sign(Algorithm.HMAC256("polaris"));
defaultJwt()
.withClaim(CLAIM_KEY_PRINCIPAL_ID, 0)
.withClaim(CLAIM_KEY_CLIENT_ID, "foo")
.sign(Algorithm.HMAC256("polaris"));
try (Response response =
newRequest("http://localhost:%d/api/management/v1/principals", newToken).get()) {
assertThat(response)
Expand Down
11 changes: 3 additions & 8 deletions polaris-service/src/test/java/org/apache/polaris/service/auth/JWTRSAKeyPairTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,12 @@
import java.util.HashMap;
import java.util.Map;
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.auth.PolarisSecretsManager.PrincipalSecretsResult;
import org.apache.polaris.core.auth.PolarisSecretsManager;
import org.apache.polaris.core.context.CallContext;
import org.apache.polaris.core.context.RealmContext;
import org.apache.polaris.core.entity.PolarisBaseEntity;
import org.apache.polaris.core.entity.PolarisEntitySubType;
import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
import org.apache.polaris.service.config.DefaultConfigurationStore;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -116,10 +115,6 @@ public void testSuccessfulTokenGeneration() throws Exception {
CallContext.setCurrentContext(getTestCallContext(polarisCallContext));
PolarisMetaStoreManager metastoreManager = Mockito.mock(PolarisMetaStoreManager.class);
String mainSecret = "client-secret";
PolarisPrincipalSecrets principalSecrets =
new PolarisPrincipalSecrets(1L, clientId, mainSecret, "otherSecret");
Mockito.when(metastoreManager.loadPrincipalSecrets(polarisCallContext, clientId))
.thenReturn(new PrincipalSecretsResult(principalSecrets));
PolarisBaseEntity principal =
new PolarisBaseEntity(
0L,
Expand All @@ -128,8 +123,8 @@ public void testSuccessfulTokenGeneration() throws Exception {
PolarisEntitySubType.NULL_SUBTYPE,
0L,
"principal");
Mockito.when(metastoreManager.loadEntity(polarisCallContext, 0L, 1L))
.thenReturn(new PolarisMetaStoreManager.EntityResult(principal));
Mockito.when(metastoreManager.validateSecret(polarisCallContext, clientId, mainSecret))
.thenReturn(new PolarisSecretsManager.SecretValidationResult(principal));
TokenBroker tokenBroker = new JWTRSAKeyPair(metastoreManager, 420);
TokenResponse token =
tokenBroker.generateFromClientSecrets(
Expand Down
11 changes: 3 additions & 8 deletions ...s-service/src/test/java/org/apache/polaris/service/auth/JWTSymmetricKeyGeneratorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,12 @@
import com.auth0.jwt.interfaces.DecodedJWT;
import java.util.Map;
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.auth.PolarisSecretsManager.PrincipalSecretsResult;
import org.apache.polaris.core.auth.PolarisSecretsManager;
import org.apache.polaris.core.context.CallContext;
import org.apache.polaris.core.context.RealmContext;
import org.apache.polaris.core.entity.PolarisBaseEntity;
import org.apache.polaris.core.entity.PolarisEntitySubType;
import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
Expand Down Expand Up @@ -63,10 +62,6 @@ public Map<String, Object> contextVariables() {
PolarisMetaStoreManager metastoreManager = Mockito.mock(PolarisMetaStoreManager.class);
String mainSecret = "test_secret";
String clientId = "test_client_id";
PolarisPrincipalSecrets principalSecrets =
new PolarisPrincipalSecrets(1L, clientId, mainSecret, "otherSecret");
Mockito.when(metastoreManager.loadPrincipalSecrets(polarisCallContext, clientId))
.thenReturn(new PrincipalSecretsResult(principalSecrets));
PolarisBaseEntity principal =
new PolarisBaseEntity(
0L,
Expand All @@ -75,8 +70,8 @@ public Map<String, Object> contextVariables() {
PolarisEntitySubType.NULL_SUBTYPE,
0L,
"principal");
Mockito.when(metastoreManager.loadEntity(polarisCallContext, 0L, 1L))
.thenReturn(new PolarisMetaStoreManager.EntityResult(principal));
Mockito.when(metastoreManager.validateSecret(polarisCallContext, clientId, mainSecret))
.thenReturn(new PolarisSecretsManager.SecretValidationResult(principal));
TokenBroker generator = new JWTSymmetricKeyBroker(metastoreManager, 666, () -> "polaris");
TokenResponse token =
generator.generateFromClientSecrets(
Expand Down

0 comments on commit 226558f

Please sign in to comment.