dimas-b commented on code in PR #2197: URL: https://github.com/apache/polaris/pull/2197#discussion_r2268007529
########## persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java: ########## @@ -774,6 +774,69 @@ public PolarisPrincipalSecrets generateNewPrincipalSecrets( return principalSecrets; } + @Nullable + @Override + public PolarisPrincipalSecrets resetPrincipalSecrets( + @Nonnull PolarisCallContext callCtx, + @Nonnull String clientId, + long principalId, + boolean reset, + @Nonnull String oldSecretHash, + String customClientId, + String customClientSecret) { + PolarisPrincipalSecrets principalSecrets = loadPrincipalSecrets(callCtx, clientId); + + // should be found + callCtx + .getDiagServices() + .checkNotNull( + principalSecrets, + "cannot_find_secrets", + "client_id={} principalId={}", + clientId, + principalId); + + // ensure principal id is matching + callCtx + .getDiagServices() + .check( + principalId == principalSecrets.getPrincipalId(), + "principal_id_mismatch", + "expectedId={} id={}", + principalId, + principalSecrets.getPrincipalId()); + + principalSecrets.setPrincipalClientId(customClientId); + principalSecrets.resetSecrets(customClientSecret); Review Comment: While that is true, I still think it is preferable to keep credential objects immutable. I do not insist on fixing prior use cases in this PR, but I'd prefer to avoid adding new code that mutates credentials in-place :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@polaris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org