You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "adutra (via GitHub)" <gi...@apache.org> on 2026/10/05 17:28:47 UTC
[PR] Move credential rotation check from authorizers to a JAX-RS filter [polaris]
adutra opened a new pull request, #5706:
URL: https://github.com/apache/polaris/pull/5706
The check that blocks principals who must rotate their credentials was duplicated in each authorizer implementation. Enforce it once in a request filter so it applies regardless of the configured authorizer.
<!--
π Describe what changes you're proposing, especially breaking or user-facing changes.
π See https://github.com/apache/polaris/blob/main/CONTRIBUTING.md for more.
-->
## Checklist
- [ ] π‘οΈ Don't disclose security issues! (contact security@apache.org)
- [ ] π Clearly explained why the changes are needed, or linked related issues: Fixes #
- [ ] π§ͺ Added/updated tests with good coverage, or manually tested (and explained how)
- [ ] π‘ Added comments for complex logic
- [ ] π§Ύ Updated `CHANGELOG.md` (if needed)
- [ ] π Updated documentation in `site/content/in-dev/unreleased` (if needed)
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Move credential rotation check from authorizers to a JAX-RS filter [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5706:
URL: https://github.com/apache/polaris/pull/5706#discussion_r4190412551
##########
runtime/service/src/main/java/org/apache/polaris/service/auth/CredentialRotationFilter.java:
##########
@@ -50,18 +54,27 @@ public void checkCredentialRotationRequired(
ContainerRequestContext requestContext, ResourceInfo resourceInfo) {
if (requestContext.getSecurityContext().getUserPrincipal() instanceof PolarisPrincipal principal
&& mustRotateCredentials(principal)
- && !isRotateCredentials(resourceInfo)
&& realmConfig.getConfig(
- FeatureConfiguration.ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING)) {
+ FeatureConfiguration.ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING)
+ && !isRotateCredentials(resourceInfo)) {
throw new ForbiddenException(
"Principal '%s' is not authorized because it must rotate credentials first",
principal.getName());
}
}
private static boolean isRotateCredentials(ResourceInfo resourceInfo) {
- return PolarisPrincipalsApi.class.isAssignableFrom(resourceInfo.getResourceClass())
- && resourceInfo.getResourceMethod().getName().equals(ROTATE_CREDENTIALS_METHOD);
+ return ROTATE_CREDENTIALS.equals(resourceInfo.getResourceMethod());
Review Comment:
optional: Is name-only comparison preferable perhaps? Anything called `rotateCredentials` in the API probably fits the use case for this filter.
Alternatively, we could use a custom annotation instead to put on relevant methods in `PolarisPrincipalsApi`
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Move credential rotation check from authorizers to a JAX-RS filter [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #5706:
URL: https://github.com/apache/polaris/pull/5706#discussion_r4188138234
##########
runtime/service/src/main/java/org/apache/polaris/service/auth/CredentialRotationFilter.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.auth;
+
+import jakarta.inject.Inject;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ResourceInfo;
+import org.apache.iceberg.exceptions.ForbiddenException;
+import org.apache.polaris.core.auth.PolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisPrincipalAttributes;
+import org.apache.polaris.core.config.FeatureConfiguration;
+import org.apache.polaris.core.config.RealmConfig;
+import org.apache.polaris.core.entity.PolarisEntityConstants;
+import org.apache.polaris.core.entity.PrincipalEntity;
+import org.apache.polaris.service.admin.api.PolarisPrincipalsApi;
+import org.apache.polaris.service.config.FilterPriorities;
+import org.jboss.resteasy.reactive.server.ServerRequestFilter;
+
+/**
+ * Rejects every request made by a principal that must rotate its credentials first, except the
+ * request that rotates them. The check is independent of the configured authorizer.
+ *
+ * <p>The check is only active when {@link
+ * FeatureConfiguration#ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING} is enabled.
+ */
+public class CredentialRotationFilter {
+
+ private static final String ROTATE_CREDENTIALS_METHOD = "rotateCredentials";
+
+ @Inject RealmConfig realmConfig;
+
+ @ServerRequestFilter(priority = FilterPriorities.CREDENTIAL_ROTATION_FILTER)
+ public void checkCredentialRotationRequired(
+ ContainerRequestContext requestContext, ResourceInfo resourceInfo) {
+ if (requestContext.getSecurityContext().getUserPrincipal() instanceof PolarisPrincipal principal
+ && mustRotateCredentials(principal)
+ && !isRotateCredentials(resourceInfo)
+ && realmConfig.getConfig(
+ FeatureConfiguration.ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING)) {
Review Comment:
Implemented, although highly unlikely.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Move credential rotation check from authorizers to a JAX-RS filter [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5706:
URL: https://github.com/apache/polaris/pull/5706#discussion_r4187088859
##########
runtime/service/src/main/java/org/apache/polaris/service/auth/DefaultPolarisAuthorizerFactory.java:
##########
@@ -32,6 +32,6 @@ class DefaultPolarisAuthorizerFactory implements PolarisAuthorizerFactory {
@Override
public PolarisAuthorizer create(RealmConfig realmConfig) {
- return new PolarisAuthorizerImpl(realmConfig);
+ return new PolarisAuthorizerImpl();
Review Comment:
nice π
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Move credential rotation check from authorizers to a JAX-RS filter [polaris]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #5706:
URL: https://github.com/apache/polaris/pull/5706#discussion_r4187002787
##########
runtime/service/src/main/java/org/apache/polaris/service/auth/CredentialRotationFilter.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.auth;
+
+import jakarta.inject.Inject;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ResourceInfo;
+import org.apache.iceberg.exceptions.ForbiddenException;
+import org.apache.polaris.core.auth.PolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisPrincipalAttributes;
+import org.apache.polaris.core.config.FeatureConfiguration;
+import org.apache.polaris.core.config.RealmConfig;
+import org.apache.polaris.core.entity.PolarisEntityConstants;
+import org.apache.polaris.core.entity.PrincipalEntity;
+import org.apache.polaris.service.admin.api.PolarisPrincipalsApi;
+import org.apache.polaris.service.config.FilterPriorities;
+import org.jboss.resteasy.reactive.server.ServerRequestFilter;
+
+/**
+ * Rejects every request made by a principal that must rotate its credentials first, except the
+ * request that rotates them. The check is independent of the configured authorizer.
+ *
+ * <p>The check is only active when {@link
+ * FeatureConfiguration#ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING} is enabled.
+ */
+public class CredentialRotationFilter {
+
+ private static final String ROTATE_CREDENTIALS_METHOD = "rotateCredentials";
+
+ @Inject RealmConfig realmConfig;
+
+ @ServerRequestFilter(priority = FilterPriorities.CREDENTIAL_ROTATION_FILTER)
+ public void checkCredentialRotationRequired(
+ ContainerRequestContext requestContext, ResourceInfo resourceInfo) {
+ if (requestContext.getSecurityContext().getUserPrincipal() instanceof PolarisPrincipal principal
+ && mustRotateCredentials(principal)
+ && !isRotateCredentials(resourceInfo)
+ && realmConfig.getConfig(
+ FeatureConfiguration.ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING)) {
Review Comment:
This condition can throw a NullPointerException when `resourceInfo` is null: `isRotateCredentials(resourceInfo)` is evaluated before the feature flag check. That means even with enforcement disabled (flag false), a rotation-required principal could still NPE if `resourceInfo` is unexpectedly null. Fix by guarding against `resourceInfo == null` inside `isRotateCredentials(...)` (or before calling it) and/or short-circuiting on the feature flag earlier (e.g., return early when the flag is disabled) so `isRotateCredentials` is never evaluated in that case.
##########
runtime/service/src/main/java/org/apache/polaris/service/auth/CredentialRotationFilter.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.auth;
+
+import jakarta.inject.Inject;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ResourceInfo;
+import org.apache.iceberg.exceptions.ForbiddenException;
+import org.apache.polaris.core.auth.PolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisPrincipalAttributes;
+import org.apache.polaris.core.config.FeatureConfiguration;
+import org.apache.polaris.core.config.RealmConfig;
+import org.apache.polaris.core.entity.PolarisEntityConstants;
+import org.apache.polaris.core.entity.PrincipalEntity;
+import org.apache.polaris.service.admin.api.PolarisPrincipalsApi;
+import org.apache.polaris.service.config.FilterPriorities;
+import org.jboss.resteasy.reactive.server.ServerRequestFilter;
+
+/**
+ * Rejects every request made by a principal that must rotate its credentials first, except the
+ * request that rotates them. The check is independent of the configured authorizer.
+ *
+ * <p>The check is only active when {@link
+ * FeatureConfiguration#ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING} is enabled.
+ */
+public class CredentialRotationFilter {
+
+ private static final String ROTATE_CREDENTIALS_METHOD = "rotateCredentials";
Review Comment:
Using a hard-coded method name string to identify the rotate-credentials endpoint is brittle (renames/overloads can silently change behavior). A more robust approach is to compare against a specific `Method` reference (resolved once), check for a dedicated marker annotation on the rotate endpoint, or validate via JAX-RS metadata (e.g., `@Path` + HTTP method) so the filter remains correct through refactors.
##########
runtime/service/src/main/java/org/apache/polaris/service/auth/CredentialRotationFilter.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.auth;
+
+import jakarta.inject.Inject;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ResourceInfo;
+import org.apache.iceberg.exceptions.ForbiddenException;
+import org.apache.polaris.core.auth.PolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisPrincipalAttributes;
+import org.apache.polaris.core.config.FeatureConfiguration;
+import org.apache.polaris.core.config.RealmConfig;
+import org.apache.polaris.core.entity.PolarisEntityConstants;
+import org.apache.polaris.core.entity.PrincipalEntity;
+import org.apache.polaris.service.admin.api.PolarisPrincipalsApi;
+import org.apache.polaris.service.config.FilterPriorities;
+import org.jboss.resteasy.reactive.server.ServerRequestFilter;
+
+/**
+ * Rejects every request made by a principal that must rotate its credentials first, except the
+ * request that rotates them. The check is independent of the configured authorizer.
+ *
+ * <p>The check is only active when {@link
+ * FeatureConfiguration#ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING} is enabled.
+ */
+public class CredentialRotationFilter {
+
+ private static final String ROTATE_CREDENTIALS_METHOD = "rotateCredentials";
+
+ @Inject RealmConfig realmConfig;
+
+ @ServerRequestFilter(priority = FilterPriorities.CREDENTIAL_ROTATION_FILTER)
+ public void checkCredentialRotationRequired(
+ ContainerRequestContext requestContext, ResourceInfo resourceInfo) {
+ if (requestContext.getSecurityContext().getUserPrincipal() instanceof PolarisPrincipal principal
+ && mustRotateCredentials(principal)
+ && !isRotateCredentials(resourceInfo)
+ && realmConfig.getConfig(
+ FeatureConfiguration.ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING)) {
+ throw new ForbiddenException(
+ "Principal '%s' is not authorized because it must rotate credentials first",
+ principal.getName());
+ }
+ }
+
+ private static boolean isRotateCredentials(ResourceInfo resourceInfo) {
+ return PolarisPrincipalsApi.class.isAssignableFrom(resourceInfo.getResourceClass())
+ && resourceInfo.getResourceMethod().getName().equals(ROTATE_CREDENTIALS_METHOD);
+ }
Review Comment:
Using a hard-coded method name string to identify the rotate-credentials endpoint is brittle (renames/overloads can silently change behavior). A more robust approach is to compare against a specific `Method` reference (resolved once), check for a dedicated marker annotation on the rotate endpoint, or validate via JAX-RS metadata (e.g., `@Path` + HTTP method) so the filter remains correct through refactors.
##########
runtime/service/src/test/java/org/apache/polaris/service/auth/CredentialRotationFilterTest.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.auth;
+
+import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ResourceInfo;
+import jakarta.ws.rs.core.SecurityContext;
+import java.lang.reflect.Method;
+import java.security.Principal;
+import java.util.Arrays;
+import java.util.Set;
+import org.apache.iceberg.exceptions.ForbiddenException;
+import org.apache.polaris.core.auth.PolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisPrincipalAttributes;
+import org.apache.polaris.core.collection.AttributeMap;
+import org.apache.polaris.core.collection.ImmutableAttributeMap;
+import org.apache.polaris.core.config.FeatureConfiguration;
+import org.apache.polaris.core.config.RealmConfig;
+import org.apache.polaris.core.entity.PrincipalEntity;
+import org.apache.polaris.service.admin.api.PolarisPrincipalsApi;
+import org.junit.jupiter.api.Test;
+
+class CredentialRotationFilterTest {
+
+ private static final PolarisPrincipal MUST_ROTATE =
+ PolarisPrincipal.of(
+ "alice",
+ ImmutableAttributeMap.builder()
+ .put(
+ PolarisPrincipalAttributes.PRINCIPAL_ENTITY_ATTRIBUTE_KEY,
+ new PrincipalEntity.Builder()
+ .setName("alice")
+ .setCredentialRotationRequiredState()
+ .build())
+ .build(),
+ Set.of("role"));
+
+ private static final PolarisPrincipal REGULAR =
+ PolarisPrincipal.of("bob", AttributeMap.EMPTY, Set.of("role"));
+
+ @Test
+ void rejectsOtherEndpoints() {
+ assertThatThrownBy(() -> filter(true, MUST_ROTATE, "listPrincipals"))
+ .isInstanceOf(ForbiddenException.class)
+ .hasMessageContaining("must rotate credentials first");
+ }
+
+ @Test
+ void allowsRotateCredentialsEndpoint() {
+ assertThatCode(() -> filter(true, MUST_ROTATE, "rotateCredentials")).doesNotThrowAnyException();
+ }
+
+ @Test
+ void allowsWhenEnforcementDisabled() {
+ assertThatCode(() -> filter(false, MUST_ROTATE, "listPrincipals")).doesNotThrowAnyException();
+ }
+
+ @Test
+ void allowsPrincipalThatDoesNotNeedToRotate() {
+ assertThatCode(() -> filter(true, REGULAR, "listPrincipals")).doesNotThrowAnyException();
+ }
+
+ @Test
+ void ignoresUnauthenticatedRequests() {
+ ContainerRequestContext requestContext = requestContext(mock(Principal.class));
+ assertThatCode(() -> newFilter(true).checkCredentialRotationRequired(requestContext, null))
+ .doesNotThrowAnyException();
+ }
+
Review Comment:
This test name says \"unauthenticated\", but it supplies a non-null `Principal` (just not a `PolarisPrincipal`). Consider adding/adjusting coverage to include (1) truly unauthenticated requests (`getUserPrincipal()` returns null) and (2) the `resourceInfo == null` case when the user principal *is* a rotation-required `PolarisPrincipal`, especially with enforcement disabledβthis would catch the current NPE risk in the filter logic.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Move credential rotation check from authorizers to a JAX-RS filter [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #5706:
URL: https://github.com/apache/polaris/pull/5706#discussion_r4194783825
##########
runtime/service/src/main/java/org/apache/polaris/service/auth/CredentialRotationFilter.java:
##########
@@ -50,18 +54,27 @@ public void checkCredentialRotationRequired(
ContainerRequestContext requestContext, ResourceInfo resourceInfo) {
if (requestContext.getSecurityContext().getUserPrincipal() instanceof PolarisPrincipal principal
&& mustRotateCredentials(principal)
- && !isRotateCredentials(resourceInfo)
&& realmConfig.getConfig(
- FeatureConfiguration.ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING)) {
+ FeatureConfiguration.ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING)
+ && !isRotateCredentials(resourceInfo)) {
throw new ForbiddenException(
"Principal '%s' is not authorized because it must rotate credentials first",
principal.getName());
}
}
private static boolean isRotateCredentials(ResourceInfo resourceInfo) {
- return PolarisPrincipalsApi.class.isAssignableFrom(resourceInfo.getResourceClass())
- && resourceInfo.getResourceMethod().getName().equals(ROTATE_CREDENTIALS_METHOD);
+ return ROTATE_CREDENTIALS.equals(resourceInfo.getResourceMethod());
Review Comment:
I initially had name-only comparison, then Copilot insisted on checking the full method signature π
In the end both are OK imo, because `rotateCredentials` is the operation ID for that endpoint, so it's (very) unlikely that it will change. The slight advantage of checking the full signature is that if the operation ID ever changes, the error pops up immediately at startup.
The annotation approach is elegant, but requires changing the mustache templates. I didn't want to go down that route.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Move credential rotation check from authorizers to a JAX-RS filter [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra merged PR #5706:
URL: https://github.com/apache/polaris/pull/5706
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Move credential rotation check from authorizers to a JAX-RS filter [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5706:
URL: https://github.com/apache/polaris/pull/5706#discussion_r4197390531
##########
runtime/service/src/main/java/org/apache/polaris/service/auth/CredentialRotationFilter.java:
##########
@@ -50,18 +54,27 @@ public void checkCredentialRotationRequired(
ContainerRequestContext requestContext, ResourceInfo resourceInfo) {
if (requestContext.getSecurityContext().getUserPrincipal() instanceof PolarisPrincipal principal
&& mustRotateCredentials(principal)
- && !isRotateCredentials(resourceInfo)
&& realmConfig.getConfig(
- FeatureConfiguration.ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING)) {
+ FeatureConfiguration.ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING)
+ && !isRotateCredentials(resourceInfo)) {
throw new ForbiddenException(
"Principal '%s' is not authorized because it must rotate credentials first",
principal.getName());
}
}
private static boolean isRotateCredentials(ResourceInfo resourceInfo) {
- return PolarisPrincipalsApi.class.isAssignableFrom(resourceInfo.getResourceClass())
- && resourceInfo.getResourceMethod().getName().equals(ROTATE_CREDENTIALS_METHOD);
+ return ROTATE_CREDENTIALS.equals(resourceInfo.getResourceMethod());
Review Comment:
fair enough :+1:
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org