comments

Author: fivetran-arunsuri

persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java

@@ -756,8 +756,7 @@ public PolarisPrincipalSecrets resetPrincipalSecrets(
       @Nonnull String clientId,
       long principalId,
       String customClientId,
-      String customClientSecret,
-      boolean customReset) {
+      String customClientSecret) {
     throw new UnsupportedOperationException(
         "This method is not supported for EclipseLink as metastore");
   }

persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java

@@ -780,8 +780,7 @@ public PolarisPrincipalSecrets resetPrincipalSecrets(
       @Nonnull String clientId,
       long principalId,
       String customClientId,
-      String customClientSecret,
-      boolean customReset) {
+      String customClientSecret) {
     PolarisPrincipalSecrets principalSecrets = loadPrincipalSecrets(callCtx, clientId);
 
     // should be found
@@ -804,7 +803,7 @@ public PolarisPrincipalSecrets resetPrincipalSecrets(
             principalId,
             principalSecrets.getPrincipalId());
 
-    if (customReset) {
+    if (customClientId != null && customClientSecret != null) {
       principalSecrets =
           new PolarisPrincipalSecrets(
               principalSecrets.getPrincipalId(), customClientId, customClientSecret, null);

polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java

@@ -912,15 +912,13 @@ private void revokeGrantRecord(
     }
 
     PolarisBaseEntity principal = loadEntityResult.getEntity();
-    Map<String, String> internalProps =
-        PolarisObjectMapperUtil.deserializeProperties(
-            principal.getInternalProperties() == null ? "{}" : principal.getInternalProperties());
+    Map<String, String> internalProps = principal.getInternalPropertiesAsMap();
 
     boolean customReset = customClientId != null && customClientSecret != null;
     PolarisPrincipalSecrets secrets =
         ((IntegrationPersistence) ms)
             .resetPrincipalSecrets(
-                callCtx, clientId, principalId, customClientId, customClientSecret, customReset);
+                callCtx, clientId, principalId, customClientId, customClientSecret);
 
     PolarisBaseEntity.Builder principalBuilder = new PolarisBaseEntity.Builder(principal);
     principalBuilder.internalProperties(PolarisObjectMapperUtil.serializeProperties(internalProps));
@@ -929,8 +927,8 @@ private void revokeGrantRecord(
       principalBuilder.entityVersion(principal.getEntityVersion());
     } else {
       principalBuilder.entityVersion(principal.getEntityVersion() + 1);
+      ms.writeEntity(callCtx, principalBuilder.build(), true, principal);
     }
-    ms.writeEntity(callCtx, principalBuilder.build(), true, principal);
     return (secrets == null)
         ? new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null)
         : new PrincipalSecretsResult(secrets);

polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java

@@ -90,16 +90,14 @@ PolarisPrincipalSecrets rotatePrincipalSecrets(
    * @param principalId principal id
    * @param customClientId the principal secret's old main secret hash
    * @param customClientSecret the principal secret's old main secret hash
-   * @param customReset
    */
   @Nullable
   PolarisPrincipalSecrets resetPrincipalSecrets(
       @Nonnull PolarisCallContext callCtx,
       @Nonnull String clientId,
       long principalId,
       String customClientId,
-      String customClientSecret,
-      boolean customReset);
+      String customClientSecret);
 
   /**
    * When dropping a principal, we also need to drop the secrets of that principal

polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java

@@ -946,7 +946,7 @@ private void bootstrapPolarisService(
 
     PolarisPrincipalSecrets secrets =
         ms.resetPrincipalSecrets(
-            callCtx, clientId, principalId, customClientId, customClientSecret, customReset);
+            callCtx, clientId, principalId, customClientId, customClientSecret);
     principalBuilder.internalProperties(PolarisObjectMapperUtil.serializeProperties(internalProps));
     // To avoid incrementing entity version twice
     if (customReset) {

polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java

@@ -500,8 +500,7 @@ public int lookupEntityGrantRecordsVersionInCurrentTxn(
       @Nonnull String clientId,
       long principalId,
       String customClientId,
-      String customClientSecret,
-      boolean customReset) {
+      String customClientSecret) {
     PolarisPrincipalSecrets principalSecrets = this.store.getSlicePrincipalSecrets().read(clientId);
 
     // should be found
@@ -525,7 +524,7 @@ public int lookupEntityGrantRecordsVersionInCurrentTxn(
             principalSecrets.getPrincipalId());
 
     // reset the secrets
-    if (customReset) {
+    if (customClientId != null && customClientSecret != null) {
       principalSecrets =
           new PolarisPrincipalSecrets(
               principalSecrets.getPrincipalId(), customClientId, customClientSecret, null);

runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java

@@ -170,15 +170,15 @@ public Response createCatalog(
     return Response.status(Response.Status.CREATED).build();
   }
 
-  private void validateResetClientId(String clientId) {
+  private void validateClientId(String clientId) {
     if (!clientId.matches("^[0-9a-f]{16}$")) {
-      throw new IllegalArgumentException("Invalid resetClientId format");
+      throw new IllegalArgumentException("Invalid clientId format");
     }
   }
 
-  private void validateResetClientSecret(String clientSecret) {
+  private void validateClientSecret(String clientSecret) {
     if (!clientSecret.matches("^[0-9a-f]{32}$")) {
-      throw new IllegalArgumentException("Invalid resetClientSecret format");
+      throw new IllegalArgumentException("Invalid clientSecret format");
     }
   }
 
@@ -307,10 +307,10 @@ public Response resetCredentials(
             : new ResetPrincipalRequest(null, null);
 
     if (safeResetPrincipalRequest.getClientId() != null) {
-      validateResetClientId(safeResetPrincipalRequest.getClientId());
+      validateClientId(safeResetPrincipalRequest.getClientId());
     }
     if (safeResetPrincipalRequest.getClientSecret() != null) {
-      validateResetClientSecret(safeResetPrincipalRequest.getClientSecret());
+      validateClientSecret(safeResetPrincipalRequest.getClientSecret());
     }
     PolarisAdminService adminService = newAdminService(realmContext, securityContext);
     return Response.ok(adminService.resetCredentials(principalName, safeResetPrincipalRequest))

spec/polaris-management-service.yml

@@ -199,7 +199,6 @@ paths:
         404:
           description: "The catalog or principal does not exist"
 
-
     put:
       operationId: updatePrincipal
       description: Update an existing principal