dimas-b commented on code in PR #3276:
URL: https://github.com/apache/polaris/pull/3276#discussion_r2621042079
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java:
##########
@@ -137,13 +137,15 @@ public Response createCatalog(
}
private void validateClientId(String clientId) {
- if (!clientId.matches("^[0-9a-f]{16}$")) {
+ if (!clientId.matches(
+
realmConfig.getConfig(FeatureConfiguration.CREDENTIAL_RESET_CLIENT_ID_PATTERN)))
{
throw new IllegalArgumentException("Invalid clientId format");
}
}
private void validateClientSecret(String clientSecret) {
- if (!clientSecret.matches("^[0-9a-f]{32}$")) {
+ if (!clientSecret.matches(
+
realmConfig.getConfig(FeatureConfiguration.CREDENTIAL_RESET_CLIENT_SECRET_PATTERN)))
{
Review Comment:
If validating client ID/secret format is becoming a nuisance to users, from
my POV this check can just be removed completely. I do not think Polaris code
relies on these values following a particular format.
If backward compatibility is a concern, I'd rather add a simple boolean flag
to _disable_ these checks (defaulting to enabled). WDYT?
##########
polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfiguration.java:
##########
@@ -34,9 +32,6 @@
* @param <T> The type of the configuration
*/
public abstract class PolarisConfiguration<T> {
-
- private static final Logger LOGGER =
LoggerFactory.getLogger(PolarisConfiguration.class);
Review Comment:
nit: if you find "dead code", it's generally preferable to cleanup in a
dedicated PR and avoid mixing cleanup with feature changes :)
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java:
##########
@@ -137,13 +137,15 @@ public Response createCatalog(
}
private void validateClientId(String clientId) {
- if (!clientId.matches("^[0-9a-f]{16}$")) {
+ if (!clientId.matches(
+
realmConfig.getConfig(FeatureConfiguration.CREDENTIAL_RESET_CLIENT_ID_PATTERN)))
{
Review Comment:
we should validate the value of `CREDENTIAL_RESET_CLIENT_ID_PATTERN` on
startup to avoid deferred RegEx syntax errors (it's an admin mistake, not the
API client's mistake)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
