You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "collado-mike (via GitHub)" <gi...@apache.org> on 2025/01/07 22:41:46 UTC
[PR] Change authentication workflow to lookup principal roles using security context [polaris]
collado-mike opened a new pull request, #623:
URL: https://github.com/apache/polaris/pull/623
<!--
Possible security vulnerabilities: STOP here and contact security@apache.org instead!
Please update the title of the PR with a meaningful message - do not leave it "empty" or "generated"
Please update this summary field:
The summary should cover these topics, if applicable:
* the motivation for the change
* a description of the status quo, for example the current behavior
* the desired behavior
* etc
PR checklist:
- Do a self-review of your code before opening a pull request
- Make sure that there's good test coverage for the changes included in this PR
- Run tests locally before pushing a PR (./gradlew check)
- Code should have comments where applicable. Particularly hard-to-understand
areas deserve good in-line documentation.
- Include changes and enhancements to the documentation (in site/content/in-dev/unreleased)
- For Work In Progress Pull Requests, please use the Draft PR feature.
Make sure to add the information BELOW this comment.
Everything in this comment will NOT be added to the PR description.
-->
Currently, the `Resolver` checks the PrincipalRole usage grants available for a Principal entity when determining which roles are active for a request. These roles are then passed on to the `PolarisAuthorizer` so that authorization can be done on the operation and target object.
This PR changes the `Resolver` to instead rely on the `SecurityContext` to determine which roles the principal has access to. A request filter populates the `SecurityContext` using an `ActiveRolesProvider`. The default implementation still looks up the principal role usage grants, but now the filter can be replaced by new implementations that determine role membership in other ways (e.g., by checking an authentication token or delegating to a third party identity provider).
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1907425816
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/PolarisPrincipalRoleSecurityContextProvider.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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.dropwizard.auth;
+
+import jakarta.annotation.Priority;
+import jakarta.inject.Inject;
+import jakarta.inject.Provider;
+import jakarta.ws.rs.Priorities;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.core.SecurityContext;
+import java.io.IOException;
+import java.security.Principal;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.apache.polaris.service.auth.BasePolarisAuthenticator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Priority(Priorities.AUTHENTICATION + 1)
+public class PolarisPrincipalRoleSecurityContextProvider implements ContainerRequestFilter {
+ private static final Logger LOGGER =
+ LoggerFactory.getLogger(PolarisPrincipalRoleSecurityContextProvider.class);
+ @Inject Provider<SecurityContext> securityContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManager;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public void filter(ContainerRequestContext requestContext) throws IOException {
+ AuthenticatedPolarisPrincipal polarisPrincipal =
+ (AuthenticatedPolarisPrincipal) securityContextProvider.get().getUserPrincipal();
Review Comment:
Isn't this simpler?
```suggestion
(AuthenticatedPolarisPrincipal) requestContext.getSecurityContext().getUserPrincipal();
```
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1907579274
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/PolarisPrincipalRoleSecurityContextProvider.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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.dropwizard.auth;
+
+import jakarta.annotation.Priority;
+import jakarta.inject.Inject;
+import jakarta.inject.Provider;
+import jakarta.ws.rs.Priorities;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.core.SecurityContext;
+import java.io.IOException;
+import java.security.Principal;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.apache.polaris.service.auth.BasePolarisAuthenticator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Priority(Priorities.AUTHENTICATION + 1)
+public class PolarisPrincipalRoleSecurityContextProvider implements ContainerRequestFilter {
+ private static final Logger LOGGER =
+ LoggerFactory.getLogger(PolarisPrincipalRoleSecurityContextProvider.class);
+ @Inject Provider<SecurityContext> securityContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManager;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public void filter(ContainerRequestContext requestContext) throws IOException {
+ AuthenticatedPolarisPrincipal polarisPrincipal =
+ (AuthenticatedPolarisPrincipal) securityContextProvider.get().getUserPrincipal();
+ if (polarisPrincipal == null) {
+ return;
+ }
+ SecurityContext securityContext =
+ createSecurityContext(requestContext.getSecurityContext(), polarisPrincipal);
+ requestContext.setSecurityContext(securityContext);
Review Comment:
It's not that uncommon to augment the security context, it's already being done (have a look at `io.dropwizard.auth.AuthFilter`). Even in Quarkus it's common practice:
https://quarkus.io/guides/security-customization#jaxrs-security-context
That said, I agree that there is a lot of overlap between `AuthenticatedPolarisPrincipal` and `SecurityContext`. I'd note that `SecurityContext` is already being passed as a method argument to all API and service methods, and from the security context you can get the principal (but not the other way around).
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1907731657
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/PolarisApplication.java:
##########
@@ -478,6 +479,52 @@ private static OpenTelemetry setupTracing() {
.build();
}
+ @jakarta.ws.rs.ext.Provider
+ @Priority(Priorities.AUTHENTICATION)
+ private static class PolarisPrincipalAuthenticator implements ContainerRequestFilter {
+ @Inject private Authenticator<String, AuthenticatedPolarisPrincipal> authenticator;
+
+ @Override
+ public void filter(ContainerRequestContext requestContext) throws IOException {
+ String authHeader = requestContext.getHeaderString("Authorization");
+ if (authHeader == null) {
+ throw new IOException("Authorization header is missing");
+ }
+ int spaceIdx = authHeader.indexOf(' ');
+ if (spaceIdx <= 0 || !authHeader.substring(0, spaceIdx).equalsIgnoreCase("Bearer")) {
+ throw new IOException("Authorization header is not a Bearer token");
Review Comment:
Same: why not `NotAuthorizedException`?
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/PolarisApplication.java:
##########
@@ -478,6 +479,52 @@ private static OpenTelemetry setupTracing() {
.build();
}
+ @jakarta.ws.rs.ext.Provider
+ @Priority(Priorities.AUTHENTICATION)
+ private static class PolarisPrincipalAuthenticator implements ContainerRequestFilter {
+ @Inject private Authenticator<String, AuthenticatedPolarisPrincipal> authenticator;
+
+ @Override
+ public void filter(ContainerRequestContext requestContext) throws IOException {
+ String authHeader = requestContext.getHeaderString("Authorization");
+ if (authHeader == null) {
+ throw new IOException("Authorization header is missing");
Review Comment:
Why not `NotAuthorizedException`?
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "collado-mike (via GitHub)" <gi...@apache.org>.
collado-mike commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1911344454
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/PolarisApplication.java:
##########
@@ -478,6 +479,52 @@ private static OpenTelemetry setupTracing() {
.build();
}
+ @jakarta.ws.rs.ext.Provider
+ @Priority(Priorities.AUTHENTICATION)
+ private static class PolarisPrincipalAuthenticator implements ContainerRequestFilter {
+ @Inject private Authenticator<String, AuthenticatedPolarisPrincipal> authenticator;
+
+ @Override
+ public void filter(ContainerRequestContext requestContext) throws IOException {
+ String authHeader = requestContext.getHeaderString("Authorization");
+ if (authHeader == null) {
+ throw new IOException("Authorization header is missing");
+ }
+ int spaceIdx = authHeader.indexOf(' ');
+ if (spaceIdx <= 0 || !authHeader.substring(0, spaceIdx).equalsIgnoreCase("Bearer")) {
+ throw new IOException("Authorization header is not a Bearer token");
Review Comment:
Originally, I'd copied this from the Dropwizard `OAuthCredentialAuthFilter`. I fixed this
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "collado-mike (via GitHub)" <gi...@apache.org>.
collado-mike commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1912201406
##########
service/common/src/main/java/org/apache/polaris/service/auth/DefaultActiveRolesProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.inject.Provider;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of the {@link ActiveRolesProvider} looks up the grant records for a
+ * principal to determine roles that are available. {@link
+ * AuthenticatedPolarisPrincipal#getActivatedPrincipalRoleNames()} is used to determine which of the
+ * available roles are active for this request.
+ */
+public class DefaultActiveRolesProvider implements ActiveRolesProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(DefaultActiveRolesProvider.class);
+ @Inject Provider<RealmContext> realmContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManagerFactory;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public Set<String> getActiveRoles(AuthenticatedPolarisPrincipal principal) {
+ List<PrincipalRoleEntity> activeRoles =
+ loadActivePrincipalRoles(
+ principal.getActivatedPrincipalRoleNames(),
+ principal.getPrincipalEntity(),
+ metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContextProvider.get()));
+ return activeRoles.stream().map(PrincipalRoleEntity::getName).collect(Collectors.toSet());
+ }
+
+ protected List<PrincipalRoleEntity> loadActivePrincipalRoles(
+ Set<String> tokenRoles, PolarisEntity principal, PolarisMetaStoreManager metaStoreManager) {
+ PolarisCallContext polarisContext = CallContext.getCurrentContext().getPolarisCallContext();
+ PolarisGrantManager.LoadGrantsResult principalGrantResults =
+ polarisGrantManagerProvider.get().loadGrantsToGrantee(polarisContext, principal);
+ polarisContext
+ .getDiagServices()
+ .check(
+ principalGrantResults.isSuccess(),
+ "Failed to resolve principal roles for principal name={} id={}",
+ principal.getName(),
+ principal.getId());
+ if (!principalGrantResults.isSuccess()) {
+ LOGGER.warn(
+ "Failed to resolve principal roles for principal name={} id={}",
+ principal.getName(),
+ principal.getId());
+ throw new NotAuthorizedException("Unable to authenticate");
+ }
+ boolean allRoles = tokenRoles.contains(BasePolarisAuthenticator.PRINCIPAL_ROLE_ALL);
+ Predicate<PrincipalRoleEntity> includeRoleFilter =
+ allRoles ? r -> true : r -> tokenRoles.contains(r.getName());
+ List<PrincipalRoleEntity> activeRoles =
+ principalGrantResults.getGrantRecords().stream()
+ .map(
+ gr ->
+ metaStoreManager.loadEntity(
+ polarisContext, gr.getSecurableCatalogId(), gr.getSecurableId()))
+ .filter(PolarisMetaStoreManager.EntityResult::isSuccess)
Review Comment:
If we can't load any grants, the call has no active roles, so no privileges will be in effect. There's no way the request can succeed in this case.
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1907564807
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java:
##########
@@ -703,61 +693,73 @@ private ResolverStatus resolvePaths(
*
* @param toValidate all entities we have resolved from the cache, hence we will have to verify
* that these entities have not changed in the backend
- * @param callerPrincipalId the id of the principal which made the call
- * @param callerPrincipalRoleNamesScope if not null, subset of roles activated by this call
* @return the status of resolution
*/
private ResolverStatus resolveCallerPrincipalAndPrincipalRoles(
- List<EntityCacheEntry> toValidate,
- long callerPrincipalId,
- String callerPrincipalName,
- Set<String> callerPrincipalRoleNamesScope) {
+ List<EntityCacheEntry> toValidate) {
// resolve the principal, by name or id
this.resolvedCallerPrincipal =
- (callerPrincipalId != PolarisEntityConstants.getNullId())
- ? this.resolveById(
- toValidate,
- PolarisEntityType.PRINCIPAL,
- PolarisEntityConstants.getNullId(),
- callerPrincipalId)
- : this.resolveByName(toValidate, PolarisEntityType.PRINCIPAL, callerPrincipalName);
+ this.resolveById(
+ toValidate,
+ PolarisEntityType.PRINCIPAL,
+ PolarisEntityConstants.getNullId(),
+ polarisPrincipal.getPrincipalEntity().getId());
// if the principal was not found, we can end right there
if (this.resolvedCallerPrincipal == null
|| this.resolvedCallerPrincipal.getEntity().isDropped()) {
return new ResolverStatus(ResolverStatus.StatusEnum.CALLER_PRINCIPAL_DOES_NOT_EXIST);
}
- // activate all principal roles which still exist
- for (PolarisGrantRecord grantRecord : this.resolvedCallerPrincipal.getGrantRecordsAsGrantee()) {
- if (grantRecord.getPrivilegeCode() == PolarisPrivilege.PRINCIPAL_ROLE_USAGE.getCode()) {
-
- // resolve principal role granted to that principal
- EntityCacheEntry principalRole =
- this.resolveById(
- toValidate,
- PolarisEntityType.PRINCIPAL_ROLE,
- PolarisEntityConstants.getNullId(),
- grantRecord.getSecurableId());
-
- // skip if purged or has been dropped
- if (principalRole != null && !principalRole.getEntity().isDropped()) {
- // add it to the activated list if no scoped principal role or this principal role is
- // activated
- if (callerPrincipalRoleNamesScope == null
- || callerPrincipalRoleNamesScope.contains(principalRole.getEntity().getName())) {
- // this principal role is activated
- this.resolvedCallerPrincipalRoles.add(principalRole);
- }
- }
- }
- }
+ // activate all principal roles specified in the authenticated principal
+ resolvedCallerPrincipalRoles =
+ this.polarisPrincipal.getActivatedPrincipalRoleNames().isEmpty()
+ ? resolveAllPrincipalRoles(toValidate, resolvedCallerPrincipal)
+ : resolvePrincipalRolesByName(
+ toValidate, this.polarisPrincipal.getActivatedPrincipalRoleNames());
// total success
return new ResolverStatus(ResolverStatus.StatusEnum.SUCCESS);
}
+ /**
+ * Resolve all principal roles that the principal has grants for
+ *
+ * @param toValidate
+ * @param resolvedCallerPrincipal1
+ * @return the list of resolved principal roles the principal has grants for
+ */
+ private List<EntityCacheEntry> resolveAllPrincipalRoles(
+ List<EntityCacheEntry> toValidate, EntityCacheEntry resolvedCallerPrincipal1) {
+ return resolvedCallerPrincipal1.getGrantRecordsAsGrantee().stream()
+ .filter(gr -> gr.getPrivilegeCode() == PolarisPrivilege.PRINCIPAL_ROLE_USAGE.getCode())
+ .map(
+ gr ->
+ resolveById(
+ toValidate,
+ PolarisEntityType.PRINCIPAL_ROLE,
+ PolarisEntityConstants.getRootEntityId(),
+ gr.getSecurableId()))
+ .collect(Collectors.toList());
+ }
+
+ /**
+ * Resolve the specified list of principal roles. The SecurityContext is used to determine whether
+ * the principal actually has the roles specified.
+ *
+ * @param toValidate
+ * @param roleNames
+ * @return the filtered list of resolved principal roles
+ */
+ private List<EntityCacheEntry> resolvePrincipalRolesByName(
+ List<EntityCacheEntry> toValidate, Set<String> roleNames) {
+ return roleNames.stream()
+ .filter(securityContext::isUserInRole)
Review Comment:
Why not use `AuthenticatedPolarisPrincipal` as the source of active role membership info? It looks like the Web App filters create both the principal and the security context, so the role information is available in the same logical scope as the principal.
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/PolarisPrincipalRoleSecurityContextProvider.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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.dropwizard.auth;
+
+import jakarta.annotation.Priority;
+import jakarta.inject.Inject;
+import jakarta.inject.Provider;
+import jakarta.ws.rs.Priorities;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.core.SecurityContext;
+import java.io.IOException;
+import java.security.Principal;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.apache.polaris.service.auth.BasePolarisAuthenticator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Priority(Priorities.AUTHENTICATION + 1)
+public class PolarisPrincipalRoleSecurityContextProvider implements ContainerRequestFilter {
+ private static final Logger LOGGER =
+ LoggerFactory.getLogger(PolarisPrincipalRoleSecurityContextProvider.class);
+ @Inject Provider<SecurityContext> securityContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManager;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public void filter(ContainerRequestContext requestContext) throws IOException {
+ AuthenticatedPolarisPrincipal polarisPrincipal =
+ (AuthenticatedPolarisPrincipal) securityContextProvider.get().getUserPrincipal();
+ if (polarisPrincipal == null) {
+ return;
+ }
+ SecurityContext securityContext =
+ createSecurityContext(requestContext.getSecurityContext(), polarisPrincipal);
+ requestContext.setSecurityContext(securityContext);
Review Comment:
Replacing the security context looks a bit too intrusive to me. I wonder if we could avoid that by using `AuthenticatedPolarisPrincipal` to convey active role membership?
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1907570720
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/PolarisPrincipalRoleSecurityContextProvider.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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.dropwizard.auth;
+
+import jakarta.annotation.Priority;
+import jakarta.inject.Inject;
+import jakarta.inject.Provider;
+import jakarta.ws.rs.Priorities;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.core.SecurityContext;
+import java.io.IOException;
+import java.security.Principal;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.apache.polaris.service.auth.BasePolarisAuthenticator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Priority(Priorities.AUTHENTICATION + 1)
+public class PolarisPrincipalRoleSecurityContextProvider implements ContainerRequestFilter {
+ private static final Logger LOGGER =
+ LoggerFactory.getLogger(PolarisPrincipalRoleSecurityContextProvider.class);
+ @Inject Provider<SecurityContext> securityContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManager;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public void filter(ContainerRequestContext requestContext) throws IOException {
+ AuthenticatedPolarisPrincipal polarisPrincipal =
+ (AuthenticatedPolarisPrincipal) securityContextProvider.get().getUserPrincipal();
+ if (polarisPrincipal == null) {
+ return;
+ }
+ SecurityContext securityContext =
+ createSecurityContext(requestContext.getSecurityContext(), polarisPrincipal);
+ requestContext.setSecurityContext(securityContext);
+ }
+
+ public SecurityContext createSecurityContext(
+ SecurityContext ctx, AuthenticatedPolarisPrincipal principal) {
+ RealmContext realmContext = CallContext.getCurrentContext().getRealmContext();
+ List<PrincipalRoleEntity> activeRoles =
+ loadActivePrincipalRoles(
+ principal.getActivatedPrincipalRoleNames(),
+ principal.getPrincipalEntity(),
+ metaStoreManager.getOrCreateMetaStoreManager(realmContext));
+ Set<String> validRoleNames =
+ activeRoles.stream().map(PrincipalRoleEntity::getName).collect(Collectors.toSet());
+ return new SecurityContext() {
+ @Override
+ public Principal getUserPrincipal() {
+ return principal;
+ }
+
+ @Override
+ public boolean isUserInRole(String role) {
+ return validRoleNames.contains(role);
+ }
+
+ @Override
+ public boolean isSecure() {
+ return ctx.isSecure();
+ }
+
+ @Override
+ public String getAuthenticationScheme() {
+ return ctx.getAuthenticationScheme();
+ }
+ };
+ }
+
+ protected List<PrincipalRoleEntity> loadActivePrincipalRoles(
+ Set<String> tokenRoles, PolarisEntity principal, PolarisMetaStoreManager metaStoreManager) {
+ PolarisCallContext polarisContext = CallContext.getCurrentContext().getPolarisCallContext();
+ PolarisGrantManager.LoadGrantsResult principalGrantResults =
+ polarisGrantManagerProvider.get().loadGrantsToGrantee(polarisContext, principal);
Review Comment:
The PR summary mentions `ActiveRolesProvider`, but I could not find it in the change set... Is it a class/interface?.. Did I miss it?
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1907617583
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/PolarisPrincipalRoleSecurityContextProvider.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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.dropwizard.auth;
+
+import jakarta.annotation.Priority;
+import jakarta.inject.Inject;
+import jakarta.inject.Provider;
+import jakarta.ws.rs.Priorities;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.core.SecurityContext;
+import java.io.IOException;
+import java.security.Principal;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.apache.polaris.service.auth.BasePolarisAuthenticator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Priority(Priorities.AUTHENTICATION + 1)
+public class PolarisPrincipalRoleSecurityContextProvider implements ContainerRequestFilter {
+ private static final Logger LOGGER =
+ LoggerFactory.getLogger(PolarisPrincipalRoleSecurityContextProvider.class);
+ @Inject Provider<SecurityContext> securityContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManager;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
Review Comment:
You don't need to inject this. The metastore manager that you obtain with `metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContext)` is also a grant manager.
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "collado-mike (via GitHub)" <gi...@apache.org>.
collado-mike commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1912203889
##########
service/common/src/main/java/org/apache/polaris/service/auth/DefaultActiveRolesProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.inject.Provider;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of the {@link ActiveRolesProvider} looks up the grant records for a
+ * principal to determine roles that are available. {@link
+ * AuthenticatedPolarisPrincipal#getActivatedPrincipalRoleNames()} is used to determine which of the
+ * available roles are active for this request.
+ */
+public class DefaultActiveRolesProvider implements ActiveRolesProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(DefaultActiveRolesProvider.class);
+ @Inject Provider<RealmContext> realmContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManagerFactory;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public Set<String> getActiveRoles(AuthenticatedPolarisPrincipal principal) {
+ List<PrincipalRoleEntity> activeRoles =
+ loadActivePrincipalRoles(
+ principal.getActivatedPrincipalRoleNames(),
+ principal.getPrincipalEntity(),
+ metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContextProvider.get()));
+ return activeRoles.stream().map(PrincipalRoleEntity::getName).collect(Collectors.toSet());
+ }
+
+ protected List<PrincipalRoleEntity> loadActivePrincipalRoles(
+ Set<String> tokenRoles, PolarisEntity principal, PolarisMetaStoreManager metaStoreManager) {
+ PolarisCallContext polarisContext = CallContext.getCurrentContext().getPolarisCallContext();
+ PolarisGrantManager.LoadGrantsResult principalGrantResults =
+ polarisGrantManagerProvider.get().loadGrantsToGrantee(polarisContext, principal);
+ polarisContext
+ .getDiagServices()
+ .check(
+ principalGrantResults.isSuccess(),
+ "Failed to resolve principal roles for principal name={} id={}",
+ principal.getName(),
+ principal.getId());
+ if (!principalGrantResults.isSuccess()) {
+ LOGGER.warn(
+ "Failed to resolve principal roles for principal name={} id={}",
+ principal.getName(),
+ principal.getId());
+ throw new NotAuthorizedException("Unable to authenticate");
Review Comment:
yeah, `NotAuthorizedException` is probably not the right one to throw here. But, as mentioned in another response, the request cannot succeed if this class returns 0 roles. So either we throw here or in the `PolarisPrincipalRoleSecurityContextProvider` - I don't see a point in continuing the request
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "collado-mike (via GitHub)" <gi...@apache.org>.
collado-mike commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1912201607
##########
service/common/src/main/java/org/apache/polaris/service/auth/DefaultActiveRolesProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.inject.Provider;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of the {@link ActiveRolesProvider} looks up the grant records for a
+ * principal to determine roles that are available. {@link
+ * AuthenticatedPolarisPrincipal#getActivatedPrincipalRoleNames()} is used to determine which of the
+ * available roles are active for this request.
+ */
+public class DefaultActiveRolesProvider implements ActiveRolesProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(DefaultActiveRolesProvider.class);
+ @Inject Provider<RealmContext> realmContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManagerFactory;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public Set<String> getActiveRoles(AuthenticatedPolarisPrincipal principal) {
+ List<PrincipalRoleEntity> activeRoles =
+ loadActivePrincipalRoles(
+ principal.getActivatedPrincipalRoleNames(),
+ principal.getPrincipalEntity(),
+ metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContextProvider.get()));
+ return activeRoles.stream().map(PrincipalRoleEntity::getName).collect(Collectors.toSet());
+ }
+
+ protected List<PrincipalRoleEntity> loadActivePrincipalRoles(
Review Comment:
no, leftover from an alternative design.
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1913281489
##########
service/common/src/main/java/org/apache/polaris/service/auth/DefaultActiveRolesProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.inject.Provider;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of the {@link ActiveRolesProvider} looks up the grant records for a
+ * principal to determine roles that are available. {@link
+ * AuthenticatedPolarisPrincipal#getActivatedPrincipalRoleNames()} is used to determine which of the
+ * available roles are active for this request.
+ */
+public class DefaultActiveRolesProvider implements ActiveRolesProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(DefaultActiveRolesProvider.class);
+ @Inject Provider<RealmContext> realmContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManagerFactory;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public Set<String> getActiveRoles(AuthenticatedPolarisPrincipal principal) {
+ List<PrincipalRoleEntity> activeRoles =
+ loadActivePrincipalRoles(
+ principal.getActivatedPrincipalRoleNames(),
+ principal.getPrincipalEntity(),
+ metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContextProvider.get()));
+ return activeRoles.stream().map(PrincipalRoleEntity::getName).collect(Collectors.toSet());
+ }
+
+ protected List<PrincipalRoleEntity> loadActivePrincipalRoles(
+ Set<String> tokenRoles, PolarisEntity principal, PolarisMetaStoreManager metaStoreManager) {
+ PolarisCallContext polarisContext = CallContext.getCurrentContext().getPolarisCallContext();
+ PolarisGrantManager.LoadGrantsResult principalGrantResults =
+ polarisGrantManagerProvider.get().loadGrantsToGrantee(polarisContext, principal);
+ polarisContext
+ .getDiagServices()
+ .check(
+ principalGrantResults.isSuccess(),
+ "Failed to resolve principal roles for principal name={} id={}",
+ principal.getName(),
+ principal.getId());
+ if (!principalGrantResults.isSuccess()) {
+ LOGGER.warn(
+ "Failed to resolve principal roles for principal name={} id={}",
+ principal.getName(),
+ principal.getId());
+ throw new NotAuthorizedException("Unable to authenticate");
+ }
+ boolean allRoles = tokenRoles.contains(BasePolarisAuthenticator.PRINCIPAL_ROLE_ALL);
+ Predicate<PrincipalRoleEntity> includeRoleFilter =
+ allRoles ? r -> true : r -> tokenRoles.contains(r.getName());
+ List<PrincipalRoleEntity> activeRoles =
+ principalGrantResults.getGrantRecords().stream()
+ .map(
+ gr ->
+ metaStoreManager.loadEntity(
+ polarisContext, gr.getSecurableCatalogId(), gr.getSecurableId()))
+ .filter(PolarisMetaStoreManager.EntityResult::isSuccess)
Review Comment:
The request can still succeed with alternative `Authorizer` implementations. I think this code should not make assumptions on how authorization is performed.
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "collado-mike (via GitHub)" <gi...@apache.org>.
collado-mike commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1912201011
##########
service/common/src/main/java/org/apache/polaris/service/auth/DefaultActiveRolesProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.inject.Provider;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of the {@link ActiveRolesProvider} looks up the grant records for a
+ * principal to determine roles that are available. {@link
+ * AuthenticatedPolarisPrincipal#getActivatedPrincipalRoleNames()} is used to determine which of the
+ * available roles are active for this request.
+ */
+public class DefaultActiveRolesProvider implements ActiveRolesProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(DefaultActiveRolesProvider.class);
+ @Inject Provider<RealmContext> realmContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManagerFactory;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public Set<String> getActiveRoles(AuthenticatedPolarisPrincipal principal) {
+ List<PrincipalRoleEntity> activeRoles =
+ loadActivePrincipalRoles(
+ principal.getActivatedPrincipalRoleNames(),
+ principal.getPrincipalEntity(),
+ metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContextProvider.get()));
+ return activeRoles.stream().map(PrincipalRoleEntity::getName).collect(Collectors.toSet());
+ }
+
+ protected List<PrincipalRoleEntity> loadActivePrincipalRoles(
+ Set<String> tokenRoles, PolarisEntity principal, PolarisMetaStoreManager metaStoreManager) {
+ PolarisCallContext polarisContext = CallContext.getCurrentContext().getPolarisCallContext();
+ PolarisGrantManager.LoadGrantsResult principalGrantResults =
+ polarisGrantManagerProvider.get().loadGrantsToGrantee(polarisContext, principal);
+ polarisContext
+ .getDiagServices()
+ .check(
+ principalGrantResults.isSuccess(),
+ "Failed to resolve principal roles for principal name={} id={}",
+ principal.getName(),
+ principal.getId());
+ if (!principalGrantResults.isSuccess()) {
+ LOGGER.warn(
+ "Failed to resolve principal roles for principal name={} id={}",
+ principal.getName(),
+ principal.getId());
+ throw new NotAuthorizedException("Unable to authenticate");
+ }
+ boolean allRoles = tokenRoles.contains(BasePolarisAuthenticator.PRINCIPAL_ROLE_ALL);
+ Predicate<PrincipalRoleEntity> includeRoleFilter =
+ allRoles ? r -> true : r -> tokenRoles.contains(r.getName());
+ List<PrincipalRoleEntity> activeRoles =
+ principalGrantResults.getGrantRecords().stream()
+ .map(
+ gr ->
+ metaStoreManager.loadEntity(
+ polarisContext, gr.getSecurableCatalogId(), gr.getSecurableId()))
+ .filter(PolarisMetaStoreManager.EntityResult::isSuccess)
+ .map(PolarisMetaStoreManager.EntityResult::getEntity)
+ .map(PrincipalRoleEntity::of)
+ .filter(includeRoleFilter)
+ .toList();
+ if (activeRoles.size() != principalGrantResults.getGrantRecords().size()) {
+ LOGGER
+ .atWarn()
+ .addKeyValue("principal", principal.getName())
+ .addKeyValue("scopes", tokenRoles)
+ .addKeyValue("roles", activeRoles)
+ .log("Some principal roles were not found in the principal's grants");
Review Comment:
I don't expect this to happen often, but including the full set of scopes and roles makes it easy to see where the gap is.
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "collado-mike (via GitHub)" <gi...@apache.org>.
collado-mike commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1911783299
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/PolarisPrincipalRoleSecurityContextProvider.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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.dropwizard.auth;
+
+import jakarta.annotation.Priority;
+import jakarta.inject.Inject;
+import jakarta.inject.Provider;
+import jakarta.ws.rs.Priorities;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.core.SecurityContext;
+import java.io.IOException;
+import java.security.Principal;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.apache.polaris.service.auth.BasePolarisAuthenticator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Priority(Priorities.AUTHENTICATION + 1)
+public class PolarisPrincipalRoleSecurityContextProvider implements ContainerRequestFilter {
+ private static final Logger LOGGER =
+ LoggerFactory.getLogger(PolarisPrincipalRoleSecurityContextProvider.class);
+ @Inject Provider<SecurityContext> securityContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManager;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public void filter(ContainerRequestContext requestContext) throws IOException {
+ AuthenticatedPolarisPrincipal polarisPrincipal =
+ (AuthenticatedPolarisPrincipal) securityContextProvider.get().getUserPrincipal();
+ if (polarisPrincipal == null) {
+ return;
+ }
+ SecurityContext securityContext =
+ createSecurityContext(requestContext.getSecurityContext(), polarisPrincipal);
+ requestContext.setSecurityContext(securityContext);
Review Comment:
TBH, I would have preferred to avoid the SecurityContext approach. However, certain details about the hk2 startup prevented me from alternative strategies, so...
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "collado-mike (via GitHub)" <gi...@apache.org>.
collado-mike commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1909216163
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/PolarisPrincipalRoleSecurityContextProvider.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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.dropwizard.auth;
+
+import jakarta.annotation.Priority;
+import jakarta.inject.Inject;
+import jakarta.inject.Provider;
+import jakarta.ws.rs.Priorities;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.core.SecurityContext;
+import java.io.IOException;
+import java.security.Principal;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.apache.polaris.service.auth.BasePolarisAuthenticator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Priority(Priorities.AUTHENTICATION + 1)
+public class PolarisPrincipalRoleSecurityContextProvider implements ContainerRequestFilter {
+ private static final Logger LOGGER =
+ LoggerFactory.getLogger(PolarisPrincipalRoleSecurityContextProvider.class);
+ @Inject Provider<SecurityContext> securityContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManager;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public void filter(ContainerRequestContext requestContext) throws IOException {
+ AuthenticatedPolarisPrincipal polarisPrincipal =
+ (AuthenticatedPolarisPrincipal) securityContextProvider.get().getUserPrincipal();
+ if (polarisPrincipal == null) {
+ return;
+ }
+ SecurityContext securityContext =
+ createSecurityContext(requestContext.getSecurityContext(), polarisPrincipal);
+ requestContext.setSecurityContext(securityContext);
+ }
+
+ public SecurityContext createSecurityContext(
+ SecurityContext ctx, AuthenticatedPolarisPrincipal principal) {
+ RealmContext realmContext = CallContext.getCurrentContext().getRealmContext();
+ List<PrincipalRoleEntity> activeRoles =
+ loadActivePrincipalRoles(
+ principal.getActivatedPrincipalRoleNames(),
+ principal.getPrincipalEntity(),
+ metaStoreManager.getOrCreateMetaStoreManager(realmContext));
+ Set<String> validRoleNames =
+ activeRoles.stream().map(PrincipalRoleEntity::getName).collect(Collectors.toSet());
+ return new SecurityContext() {
+ @Override
+ public Principal getUserPrincipal() {
+ return principal;
+ }
+
+ @Override
+ public boolean isUserInRole(String role) {
+ return validRoleNames.contains(role);
+ }
+
+ @Override
+ public boolean isSecure() {
+ return ctx.isSecure();
+ }
+
+ @Override
+ public String getAuthenticationScheme() {
+ return ctx.getAuthenticationScheme();
+ }
+ };
+ }
+
+ protected List<PrincipalRoleEntity> loadActivePrincipalRoles(
+ Set<String> tokenRoles, PolarisEntity principal, PolarisMetaStoreManager metaStoreManager) {
+ PolarisCallContext polarisContext = CallContext.getCurrentContext().getPolarisCallContext();
+ PolarisGrantManager.LoadGrantsResult principalGrantResults =
+ polarisGrantManagerProvider.get().loadGrantsToGrantee(polarisContext, principal);
Review Comment:
Shoot - a second commit didn't get pushed. I'll fix it
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "collado-mike (via GitHub)" <gi...@apache.org>.
collado-mike commented on PR #623:
URL: https://github.com/apache/polaris/pull/623#issuecomment-2584985199
@adutra @dimas-b can you re-approve? I rebased and now it requires another approval
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "collado-mike (via GitHub)" <gi...@apache.org>.
collado-mike commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1912200422
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/PolarisPrincipalAuthenticator.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.dropwizard.auth;
+
+import jakarta.annotation.Priority;
+import jakarta.inject.Inject;
+import jakarta.ws.rs.Priorities;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.core.SecurityContext;
+import java.io.IOException;
+import java.security.Principal;
+import java.util.Optional;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.service.auth.Authenticator;
+
+@jakarta.ws.rs.ext.Provider
+@Priority(Priorities.AUTHENTICATION)
+public class PolarisPrincipalAuthenticator implements ContainerRequestFilter {
+ @Inject private Authenticator<String, AuthenticatedPolarisPrincipal> authenticator;
+
+ @Override
+ public void filter(ContainerRequestContext requestContext) throws IOException {
+ String authHeader = requestContext.getHeaderString("Authorization");
+ if (authHeader == null) {
+ throw new IOException("Authorization header is missing");
+ }
+ int spaceIdx = authHeader.indexOf(' ');
+ if (spaceIdx <= 0 || !authHeader.substring(0, spaceIdx).equalsIgnoreCase("Bearer")) {
+ throw new IOException("Authorization header is not a Bearer token");
Review Comment:
Ugh. I fixed this in the previous commit.
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "collado-mike (via GitHub)" <gi...@apache.org>.
collado-mike merged PR #623:
URL: https://github.com/apache/polaris/pull/623
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1907614785
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/PolarisApplication.java:
##########
@@ -478,6 +479,52 @@ private static OpenTelemetry setupTracing() {
.build();
}
+ @jakarta.ws.rs.ext.Provider
+ @Priority(Priorities.AUTHENTICATION)
+ private static class PolarisPrincipalAuthenticator implements ContainerRequestFilter {
Review Comment:
Can you move this class to a top-level class? It will make the Quarkus adaptation easier.
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/PolarisPrincipalRoleSecurityContextProvider.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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.dropwizard.auth;
+
+import jakarta.annotation.Priority;
+import jakarta.inject.Inject;
+import jakarta.inject.Provider;
+import jakarta.ws.rs.Priorities;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.core.SecurityContext;
+import java.io.IOException;
+import java.security.Principal;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.apache.polaris.service.auth.BasePolarisAuthenticator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Priority(Priorities.AUTHENTICATION + 1)
+public class PolarisPrincipalRoleSecurityContextProvider implements ContainerRequestFilter {
+ private static final Logger LOGGER =
+ LoggerFactory.getLogger(PolarisPrincipalRoleSecurityContextProvider.class);
+ @Inject Provider<SecurityContext> securityContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManager;
Review Comment:
Let's rename to `metaStoreManagerFactory`; I find it very confusing to name this component as if it were a bean it creates.
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/PolarisPrincipalRoleSecurityContextProvider.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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.dropwizard.auth;
+
+import jakarta.annotation.Priority;
+import jakarta.inject.Inject;
+import jakarta.inject.Provider;
+import jakarta.ws.rs.Priorities;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.core.SecurityContext;
+import java.io.IOException;
+import java.security.Principal;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.apache.polaris.service.auth.BasePolarisAuthenticator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Priority(Priorities.AUTHENTICATION + 1)
+public class PolarisPrincipalRoleSecurityContextProvider implements ContainerRequestFilter {
+ private static final Logger LOGGER =
+ LoggerFactory.getLogger(PolarisPrincipalRoleSecurityContextProvider.class);
+ @Inject Provider<SecurityContext> securityContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManager;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
Review Comment:
You don't need to inject this. The metastore manager that you obtain with `metaStoreManager.getOrCreateMetaStoreManager(realmContext)` is also a grant manager.
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/PolarisPrincipalRoleSecurityContextProvider.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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.dropwizard.auth;
+
+import jakarta.annotation.Priority;
+import jakarta.inject.Inject;
+import jakarta.inject.Provider;
+import jakarta.ws.rs.Priorities;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.core.SecurityContext;
+import java.io.IOException;
+import java.security.Principal;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.apache.polaris.service.auth.BasePolarisAuthenticator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Priority(Priorities.AUTHENTICATION + 1)
+public class PolarisPrincipalRoleSecurityContextProvider implements ContainerRequestFilter {
+ private static final Logger LOGGER =
+ LoggerFactory.getLogger(PolarisPrincipalRoleSecurityContextProvider.class);
+ @Inject Provider<SecurityContext> securityContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManager;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public void filter(ContainerRequestContext requestContext) throws IOException {
+ AuthenticatedPolarisPrincipal polarisPrincipal =
+ (AuthenticatedPolarisPrincipal) securityContextProvider.get().getUserPrincipal();
+ if (polarisPrincipal == null) {
+ return;
+ }
+ SecurityContext securityContext =
+ createSecurityContext(requestContext.getSecurityContext(), polarisPrincipal);
+ requestContext.setSecurityContext(securityContext);
+ }
+
+ public SecurityContext createSecurityContext(
+ SecurityContext ctx, AuthenticatedPolarisPrincipal principal) {
+ RealmContext realmContext = CallContext.getCurrentContext().getRealmContext();
+ List<PrincipalRoleEntity> activeRoles =
+ loadActivePrincipalRoles(
+ principal.getActivatedPrincipalRoleNames(),
+ principal.getPrincipalEntity(),
+ metaStoreManager.getOrCreateMetaStoreManager(realmContext));
+ Set<String> validRoleNames =
+ activeRoles.stream().map(PrincipalRoleEntity::getName).collect(Collectors.toSet());
+ return new SecurityContext() {
+ @Override
+ public Principal getUserPrincipal() {
+ return principal;
+ }
+
+ @Override
+ public boolean isUserInRole(String role) {
+ return validRoleNames.contains(role);
+ }
+
+ @Override
+ public boolean isSecure() {
+ return ctx.isSecure();
+ }
+
+ @Override
+ public String getAuthenticationScheme() {
+ return ctx.getAuthenticationScheme();
+ }
+ };
+ }
+
+ protected List<PrincipalRoleEntity> loadActivePrincipalRoles(
+ Set<String> tokenRoles, PolarisEntity principal, PolarisMetaStoreManager metaStoreManager) {
+ PolarisCallContext polarisContext = CallContext.getCurrentContext().getPolarisCallContext();
+ PolarisGrantManager.LoadGrantsResult principalGrantResults =
+ polarisGrantManagerProvider.get().loadGrantsToGrantee(polarisContext, principal);
Review Comment:
```suggestion
metaStoreManager.loadGrantsToGrantee(polarisContext, principal);
```
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on PR #623:
URL: https://github.com/apache/polaris/pull/623#issuecomment-2585197313
> From my POV this PR can be merged with possible future improvements for the points raised in comments. I'm not approving now mostly to get @adutra 's opinion on the additional changes added after the previous round of reviews.
Agreed, there is definitely room from improvements, but the PR is mergeable as is.
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1911877115
##########
service/common/src/main/java/org/apache/polaris/service/auth/DefaultActiveRolesProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.inject.Provider;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of the {@link ActiveRolesProvider} looks up the grant records for a
+ * principal to determine roles that are available. {@link
+ * AuthenticatedPolarisPrincipal#getActivatedPrincipalRoleNames()} is used to determine which of the
+ * available roles are active for this request.
+ */
+public class DefaultActiveRolesProvider implements ActiveRolesProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(DefaultActiveRolesProvider.class);
+ @Inject Provider<RealmContext> realmContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManagerFactory;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public Set<String> getActiveRoles(AuthenticatedPolarisPrincipal principal) {
+ List<PrincipalRoleEntity> activeRoles =
+ loadActivePrincipalRoles(
+ principal.getActivatedPrincipalRoleNames(),
+ principal.getPrincipalEntity(),
+ metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContextProvider.get()));
+ return activeRoles.stream().map(PrincipalRoleEntity::getName).collect(Collectors.toSet());
+ }
+
+ protected List<PrincipalRoleEntity> loadActivePrincipalRoles(
Review Comment:
Why `protected`? There are no overrides in the Polaris codebase, and the class is not really part of public API, is it?
##########
service/common/src/main/java/org/apache/polaris/service/auth/DefaultActiveRolesProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.inject.Provider;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of the {@link ActiveRolesProvider} looks up the grant records for a
+ * principal to determine roles that are available. {@link
+ * AuthenticatedPolarisPrincipal#getActivatedPrincipalRoleNames()} is used to determine which of the
+ * available roles are active for this request.
+ */
+public class DefaultActiveRolesProvider implements ActiveRolesProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(DefaultActiveRolesProvider.class);
+ @Inject Provider<RealmContext> realmContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManagerFactory;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public Set<String> getActiveRoles(AuthenticatedPolarisPrincipal principal) {
+ List<PrincipalRoleEntity> activeRoles =
+ loadActivePrincipalRoles(
+ principal.getActivatedPrincipalRoleNames(),
+ principal.getPrincipalEntity(),
+ metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContextProvider.get()));
+ return activeRoles.stream().map(PrincipalRoleEntity::getName).collect(Collectors.toSet());
+ }
+
+ protected List<PrincipalRoleEntity> loadActivePrincipalRoles(
+ Set<String> tokenRoles, PolarisEntity principal, PolarisMetaStoreManager metaStoreManager) {
+ PolarisCallContext polarisContext = CallContext.getCurrentContext().getPolarisCallContext();
+ PolarisGrantManager.LoadGrantsResult principalGrantResults =
+ polarisGrantManagerProvider.get().loadGrantsToGrantee(polarisContext, principal);
+ polarisContext
+ .getDiagServices()
+ .check(
+ principalGrantResults.isSuccess(),
+ "Failed to resolve principal roles for principal name={} id={}",
+ principal.getName(),
+ principal.getId());
+ if (!principalGrantResults.isSuccess()) {
+ LOGGER.warn(
+ "Failed to resolve principal roles for principal name={} id={}",
+ principal.getName(),
+ principal.getId());
+ throw new NotAuthorizedException("Unable to authenticate");
+ }
+ boolean allRoles = tokenRoles.contains(BasePolarisAuthenticator.PRINCIPAL_ROLE_ALL);
+ Predicate<PrincipalRoleEntity> includeRoleFilter =
+ allRoles ? r -> true : r -> tokenRoles.contains(r.getName());
+ List<PrincipalRoleEntity> activeRoles =
+ principalGrantResults.getGrantRecords().stream()
+ .map(
+ gr ->
+ metaStoreManager.loadEntity(
+ polarisContext, gr.getSecurableCatalogId(), gr.getSecurableId()))
+ .filter(PolarisMetaStoreManager.EntityResult::isSuccess)
+ .map(PolarisMetaStoreManager.EntityResult::getEntity)
+ .map(PrincipalRoleEntity::of)
Review Comment:
Callers always use only the names of active roles... why bother converting to a different object type?
##########
service/common/src/main/java/org/apache/polaris/service/auth/DefaultActiveRolesProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.inject.Provider;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of the {@link ActiveRolesProvider} looks up the grant records for a
+ * principal to determine roles that are available. {@link
+ * AuthenticatedPolarisPrincipal#getActivatedPrincipalRoleNames()} is used to determine which of the
+ * available roles are active for this request.
+ */
+public class DefaultActiveRolesProvider implements ActiveRolesProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(DefaultActiveRolesProvider.class);
+ @Inject Provider<RealmContext> realmContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManagerFactory;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public Set<String> getActiveRoles(AuthenticatedPolarisPrincipal principal) {
+ List<PrincipalRoleEntity> activeRoles =
+ loadActivePrincipalRoles(
+ principal.getActivatedPrincipalRoleNames(),
+ principal.getPrincipalEntity(),
+ metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContextProvider.get()));
+ return activeRoles.stream().map(PrincipalRoleEntity::getName).collect(Collectors.toSet());
+ }
+
+ protected List<PrincipalRoleEntity> loadActivePrincipalRoles(
+ Set<String> tokenRoles, PolarisEntity principal, PolarisMetaStoreManager metaStoreManager) {
+ PolarisCallContext polarisContext = CallContext.getCurrentContext().getPolarisCallContext();
+ PolarisGrantManager.LoadGrantsResult principalGrantResults =
+ polarisGrantManagerProvider.get().loadGrantsToGrantee(polarisContext, principal);
+ polarisContext
+ .getDiagServices()
+ .check(
+ principalGrantResults.isSuccess(),
+ "Failed to resolve principal roles for principal name={} id={}",
+ principal.getName(),
+ principal.getId());
+ if (!principalGrantResults.isSuccess()) {
+ LOGGER.warn(
+ "Failed to resolve principal roles for principal name={} id={}",
+ principal.getName(),
+ principal.getId());
+ throw new NotAuthorizedException("Unable to authenticate");
+ }
+ boolean allRoles = tokenRoles.contains(BasePolarisAuthenticator.PRINCIPAL_ROLE_ALL);
+ Predicate<PrincipalRoleEntity> includeRoleFilter =
+ allRoles ? r -> true : r -> tokenRoles.contains(r.getName());
+ List<PrincipalRoleEntity> activeRoles =
+ principalGrantResults.getGrantRecords().stream()
+ .map(
+ gr ->
+ metaStoreManager.loadEntity(
+ polarisContext, gr.getSecurableCatalogId(), gr.getSecurableId()))
+ .filter(PolarisMetaStoreManager.EntityResult::isSuccess)
Review Comment:
Why do we treat failures to load grant records as an error (line 79) but filter out failures to load related roles?
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/PolarisPrincipalAuthenticator.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.dropwizard.auth;
+
+import jakarta.annotation.Priority;
+import jakarta.inject.Inject;
+import jakarta.ws.rs.Priorities;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.core.SecurityContext;
+import java.io.IOException;
+import java.security.Principal;
+import java.util.Optional;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.service.auth.Authenticator;
+
+@jakarta.ws.rs.ext.Provider
+@Priority(Priorities.AUTHENTICATION)
+public class PolarisPrincipalAuthenticator implements ContainerRequestFilter {
+ @Inject private Authenticator<String, AuthenticatedPolarisPrincipal> authenticator;
+
+ @Override
+ public void filter(ContainerRequestContext requestContext) throws IOException {
+ String authHeader = requestContext.getHeaderString("Authorization");
+ if (authHeader == null) {
+ throw new IOException("Authorization header is missing");
+ }
+ int spaceIdx = authHeader.indexOf(' ');
+ if (spaceIdx <= 0 || !authHeader.substring(0, spaceIdx).equalsIgnoreCase("Bearer")) {
+ throw new IOException("Authorization header is not a Bearer token");
Review Comment:
Why is this not a `NotAuthorizedException`? Ideally the response should be 401 with the `WWW-Authenticate: Bearer` header, I think.
##########
service/common/src/main/java/org/apache/polaris/service/auth/DefaultActiveRolesProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.inject.Provider;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of the {@link ActiveRolesProvider} looks up the grant records for a
+ * principal to determine roles that are available. {@link
+ * AuthenticatedPolarisPrincipal#getActivatedPrincipalRoleNames()} is used to determine which of the
+ * available roles are active for this request.
+ */
+public class DefaultActiveRolesProvider implements ActiveRolesProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(DefaultActiveRolesProvider.class);
+ @Inject Provider<RealmContext> realmContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManagerFactory;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public Set<String> getActiveRoles(AuthenticatedPolarisPrincipal principal) {
+ List<PrincipalRoleEntity> activeRoles =
+ loadActivePrincipalRoles(
+ principal.getActivatedPrincipalRoleNames(),
+ principal.getPrincipalEntity(),
+ metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContextProvider.get()));
+ return activeRoles.stream().map(PrincipalRoleEntity::getName).collect(Collectors.toSet());
+ }
+
+ protected List<PrincipalRoleEntity> loadActivePrincipalRoles(
+ Set<String> tokenRoles, PolarisEntity principal, PolarisMetaStoreManager metaStoreManager) {
+ PolarisCallContext polarisContext = CallContext.getCurrentContext().getPolarisCallContext();
+ PolarisGrantManager.LoadGrantsResult principalGrantResults =
+ polarisGrantManagerProvider.get().loadGrantsToGrantee(polarisContext, principal);
+ polarisContext
+ .getDiagServices()
+ .check(
+ principalGrantResults.isSuccess(),
+ "Failed to resolve principal roles for principal name={} id={}",
+ principal.getName(),
+ principal.getId());
+ if (!principalGrantResults.isSuccess()) {
+ LOGGER.warn(
+ "Failed to resolve principal roles for principal name={} id={}",
+ principal.getName(),
+ principal.getId());
+ throw new NotAuthorizedException("Unable to authenticate");
Review Comment:
This looks like an overreach to me... The `ActiveRolesProvider` interface does not imply that the provider makes authorization decisions.
I'd think we should either return an empty list of active roles in this case, or raise an "internal server error" instead.
Also, the message mentions "authenticate", but there was no authentication decision made in the context of this method. The input was already an `AuthenticatedPolarisPrincipal` :thinking:
##########
service/common/src/main/java/org/apache/polaris/service/auth/DefaultActiveRolesProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.inject.Provider;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of the {@link ActiveRolesProvider} looks up the grant records for a
+ * principal to determine roles that are available. {@link
+ * AuthenticatedPolarisPrincipal#getActivatedPrincipalRoleNames()} is used to determine which of the
+ * available roles are active for this request.
+ */
+public class DefaultActiveRolesProvider implements ActiveRolesProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(DefaultActiveRolesProvider.class);
+ @Inject Provider<RealmContext> realmContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManagerFactory;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public Set<String> getActiveRoles(AuthenticatedPolarisPrincipal principal) {
+ List<PrincipalRoleEntity> activeRoles =
+ loadActivePrincipalRoles(
+ principal.getActivatedPrincipalRoleNames(),
+ principal.getPrincipalEntity(),
+ metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContextProvider.get()));
+ return activeRoles.stream().map(PrincipalRoleEntity::getName).collect(Collectors.toSet());
+ }
+
+ protected List<PrincipalRoleEntity> loadActivePrincipalRoles(
+ Set<String> tokenRoles, PolarisEntity principal, PolarisMetaStoreManager metaStoreManager) {
+ PolarisCallContext polarisContext = CallContext.getCurrentContext().getPolarisCallContext();
+ PolarisGrantManager.LoadGrantsResult principalGrantResults =
+ polarisGrantManagerProvider.get().loadGrantsToGrantee(polarisContext, principal);
+ polarisContext
+ .getDiagServices()
+ .check(
+ principalGrantResults.isSuccess(),
+ "Failed to resolve principal roles for principal name={} id={}",
+ principal.getName(),
+ principal.getId());
+ if (!principalGrantResults.isSuccess()) {
+ LOGGER.warn(
+ "Failed to resolve principal roles for principal name={} id={}",
+ principal.getName(),
+ principal.getId());
+ throw new NotAuthorizedException("Unable to authenticate");
+ }
+ boolean allRoles = tokenRoles.contains(BasePolarisAuthenticator.PRINCIPAL_ROLE_ALL);
+ Predicate<PrincipalRoleEntity> includeRoleFilter =
+ allRoles ? r -> true : r -> tokenRoles.contains(r.getName());
+ List<PrincipalRoleEntity> activeRoles =
+ principalGrantResults.getGrantRecords().stream()
+ .map(
+ gr ->
+ metaStoreManager.loadEntity(
+ polarisContext, gr.getSecurableCatalogId(), gr.getSecurableId()))
+ .filter(PolarisMetaStoreManager.EntityResult::isSuccess)
+ .map(PolarisMetaStoreManager.EntityResult::getEntity)
+ .map(PrincipalRoleEntity::of)
+ .filter(includeRoleFilter)
+ .toList();
+ if (activeRoles.size() != principalGrantResults.getGrantRecords().size()) {
+ LOGGER
+ .atWarn()
+ .addKeyValue("principal", principal.getName())
+ .addKeyValue("scopes", tokenRoles)
+ .addKeyValue("roles", activeRoles)
+ .log("Some principal roles were not found in the principal's grants");
Review Comment:
Is this message helpful for trouble-shooting? Why not log individual failures?
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "collado-mike (via GitHub)" <gi...@apache.org>.
collado-mike commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1912209272
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/PolarisPrincipalAuthenticator.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.dropwizard.auth;
+
+import jakarta.annotation.Priority;
+import jakarta.inject.Inject;
+import jakarta.ws.rs.Priorities;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.core.SecurityContext;
+import java.io.IOException;
+import java.security.Principal;
+import java.util.Optional;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.service.auth.Authenticator;
+
+@jakarta.ws.rs.ext.Provider
+@Priority(Priorities.AUTHENTICATION)
+public class PolarisPrincipalAuthenticator implements ContainerRequestFilter {
+ @Inject private Authenticator<String, AuthenticatedPolarisPrincipal> authenticator;
+
+ @Override
+ public void filter(ContainerRequestContext requestContext) throws IOException {
+ String authHeader = requestContext.getHeaderString("Authorization");
+ if (authHeader == null) {
+ throw new IOException("Authorization header is missing");
+ }
+ int spaceIdx = authHeader.indexOf(' ');
+ if (spaceIdx <= 0 || !authHeader.substring(0, spaceIdx).equalsIgnoreCase("Bearer")) {
+ throw new IOException("Authorization header is not a Bearer token");
Review Comment:
my git-fu is not strong this week :( I'll push these fixes separately
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1907588656
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/PolarisPrincipalRoleSecurityContextProvider.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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.dropwizard.auth;
+
+import jakarta.annotation.Priority;
+import jakarta.inject.Inject;
+import jakarta.inject.Provider;
+import jakarta.ws.rs.Priorities;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.core.SecurityContext;
+import java.io.IOException;
+import java.security.Principal;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.apache.polaris.service.auth.BasePolarisAuthenticator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Priority(Priorities.AUTHENTICATION + 1)
+public class PolarisPrincipalRoleSecurityContextProvider implements ContainerRequestFilter {
+ private static final Logger LOGGER =
+ LoggerFactory.getLogger(PolarisPrincipalRoleSecurityContextProvider.class);
+ @Inject Provider<SecurityContext> securityContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManager;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
+
+ @Override
+ public void filter(ContainerRequestContext requestContext) throws IOException {
+ AuthenticatedPolarisPrincipal polarisPrincipal =
+ (AuthenticatedPolarisPrincipal) securityContextProvider.get().getUserPrincipal();
+ if (polarisPrincipal == null) {
+ return;
+ }
+ SecurityContext securityContext =
+ createSecurityContext(requestContext.getSecurityContext(), polarisPrincipal);
+ requestContext.setSecurityContext(securityContext);
Review Comment:
I'm not against augmenting the `SecurityContext` in principle, but it this particular situation it looks like we're turning the abstractions inside out. We need extra information in Polaris Core, while `SecurityContext` is a Web App concept. We do not actually need to alter the behaviour or other Web App components (or 3rd party code) with the new information. We only alter the behaviour of Polaris code. I think it would be logical to use Polaris classes for that purpose.
That said, I do not feel too strongly about this :)
--
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] Change authentication workflow to lookup principal roles using security context [polaris]
Posted by "collado-mike (via GitHub)" <gi...@apache.org>.
collado-mike commented on code in PR #623:
URL: https://github.com/apache/polaris/pull/623#discussion_r1911782609
##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/PolarisPrincipalRoleSecurityContextProvider.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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.dropwizard.auth;
+
+import jakarta.annotation.Priority;
+import jakarta.inject.Inject;
+import jakarta.inject.Provider;
+import jakarta.ws.rs.Priorities;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.core.SecurityContext;
+import java.io.IOException;
+import java.security.Principal;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisGrantManager;
+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.PrincipalRoleEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.apache.polaris.service.auth.BasePolarisAuthenticator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Priority(Priorities.AUTHENTICATION + 1)
+public class PolarisPrincipalRoleSecurityContextProvider implements ContainerRequestFilter {
+ private static final Logger LOGGER =
+ LoggerFactory.getLogger(PolarisPrincipalRoleSecurityContextProvider.class);
+ @Inject Provider<SecurityContext> securityContextProvider;
+ @Inject MetaStoreManagerFactory metaStoreManager;
+ @Inject Provider<PolarisGrantManager> polarisGrantManagerProvider;
Review Comment:
I'm trying to distinguish between the metastoreManager and the grantManager and the other interfaces so that we can split them more easily.
--
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