Skip to content

Refactor PolarisPrincipal to contain generic attributes #5085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adutra merged 2 commits into from
Jul 21, 2026
Merged

Refactor PolarisPrincipal to contain generic attributes #5085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4c64c1e
Refactor PolarisPrincipal to contain generic attributes
adutra Jul 17, 2026
daae953
changelog
adutra Jul 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,19 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
are now always bootstrapped with the latest available schema version.
- The `MaintenanceService.performMaintenance()` signature now requires an explicit `OptionalLong overrideRunId` argument to supersede the latest unfinished maintenance run.
- Admin grant APIs now reject table-like privilege targets with an empty namespace. A table-like target without a namespace is considered invalid input.
- `PolarisPrincipal` now carries a generic `Map<String, Object> attributes` bag instead of the
previous `Map<String, String> properties` and `Optional<String> token` fields. Three well-known
attribute keys are defined as constants on the interface:

- `PolarisPrincipal.PRINCIPAL_ENTITY_ATTRIBUTE_KEY`: holds the `PrincipalEntity`;
- `PolarisPrincipal.PRINCIPAL_ROLE_ALL_ATTRIBUTE_KEY`: holds a boolean indicating if the
`PRINCIPAL_ROLE:ALL` pseudo-role is present;
- `PolarisPrincipal.JWT_ATTRIBUTE_KEY`: holds the raw JWT string.

Use `PolarisPrincipal.getAttribute(key, type)` for type-safe access. The `Authenticator` interface
now accepts a Quarkus `SecurityIdentity` instead of a `PolarisCredential`, and throws
`AuthenticationFailedException` (mapped to HTTP 401) instead of Iceberg's
`NotAuthorizedException`.

### New Features
- Added GCS principal attribution for vended credentials (the GCP counterpart of AWS STS session tags). Set `GCS_PRINCIPAL_ATTRIBUTION_ENABLED=true` to activate; the feature flags `GCS_PRINCIPAL_ATTRIBUTION_WIF_AUDIENCE`, `GCS_PRINCIPAL_ATTRIBUTION_TOKEN_ISSUER`, and `GCS_PRINCIPAL_ATTRIBUTION_SIGNING_KEY_FILE` are then required (a missing value is a fatal configuration error). Also requires a `gcpServiceAccount` on the catalog StorageConfiguration. When enabled, credential vending chains a catalog-signed JWT through a Workload Identity Federation token exchange and service-account impersonation, so the Polaris principal appears in GCS Data Access audit logs (`serviceAccountDelegationInfo.principalSubject`) for any client. `GCS_PRINCIPAL_ATTRIBUTION_SIGNING_KEY_ID` sets the JWT `kid` for JWKS key rotation. Attribution is keyed per-principal in the credential cache; when disabled (default), GCP vending behaviour is unchanged.
Expand Down
11 changes: 7 additions & 4 deletions ...auth/ranger/src/main/java/org/apache/polaris/extension/auth/ranger/utils/RangerUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.apache.polaris.core.auth.PolarisPrincipal;
import org.apache.polaris.core.entity.PolarisEntity;
import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.entity.PrincipalEntity;
import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper;
import org.apache.polaris.core.persistence.ResolvedPolarisEntity;
import org.apache.ranger.authz.model.RangerAccessInfo;
Expand Down Expand Up @@ -127,10 +128,12 @@ private static Map<String, Object> getResourceAttributes(
}

private static Map<String, Object> getUserAttributes(PolarisPrincipal principal) {
Map<String, String> properties = principal.getProperties();
Map<String, String> properties =
principal
.getAttribute(PolarisPrincipal.PRINCIPAL_ENTITY_ATTRIBUTE_KEY, PrincipalEntity.class)
.map(PrincipalEntity::getInternalPropertiesAsMap)

@dimas-b dimas-b Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we still need #5032 to forward user-level principal properties to authorizers... It will be a follow-up change. WDYT? CC: @iprithv

@adutra adutra Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we'd need #5032, authorizers can get the user-facing properties as follows :

principal
  .getAttribute(PolarisPrincipal.PRINCIPAL_ENTITY_ATTRIBUTE_KEY, PrincipalEntity.class)
  .map(PrincipalEntity::getPropertiesAsMap)
  .orElse(Map.of());

@dimas-b dimas-b Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I mean that #5032 actually forwards user-level principal properties to OPA and Ranger (IIRC), so some extra change will be required to keep that behaviour after this PR.

.orElse(Collections.emptyMap());

return (properties == null || properties.isEmpty())
? Collections.emptyMap()
: new HashMap<>(properties);
return properties.isEmpty() ? Collections.emptyMap() : new HashMap<>(properties);
}
}
2 changes: 1 addition & 1 deletion ...main/java/org/apache/polaris/service/it/test/PolarisManagementServiceIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1028,7 +1028,7 @@ public void testCreatePrincipalAndRotateCredentials() {
.isNotNull()
.extracting(ErrorResponse::message)
.asString()
.contains("PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE");
.contains("must rotate credentials first");
}

// Now try to rotate using the principal's token.
Expand Down
18 changes: 14 additions & 4 deletions polaris-core/src/main/java/org/apache/polaris/core/auth/AuthorizationPreConditions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.polaris.core.config.FeatureConfiguration;
import org.apache.polaris.core.config.RealmConfig;
import org.apache.polaris.core.entity.PolarisEntityConstants;
import org.apache.polaris.core.entity.PrincipalEntity;

/**
* Common pre-condition checks shared across authorizer implementations for credential-related
Expand All @@ -48,12 +49,21 @@ public static void checkCredentialRotationRequired(
if (realmConfig.getConfig(
FeatureConfiguration.ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING)
&& authzOp != PolarisAuthorizableOperation.ROTATE_CREDENTIALS
&& polarisPrincipal
.getProperties()
.containsKey(PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE)) {
&& mustRotateCredentials(polarisPrincipal)) {
throw new ForbiddenException(
"Principal '%s' is not authorized for op %s due to PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE",
"Principal '%s' is not authorized for op %s because it must rotate credentials first",
polarisPrincipal.getName(), authzOp);
}
}

private static boolean mustRotateCredentials(PolarisPrincipal principal) {

@flyrain flyrain Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PolarisPrincipal's own javadoc says callers must never assume this attribute is present. But the new logic here mandate the property PRINCIPAL_ENTITY_ATTRIBUTE_KEY, otherwise, rotation will skip. I'm not sure the new behavior is correct. Maybe we should make PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE the first level property instead of embedded in PRINCIPAL_ENTITY_ATTRIBUTE_KEY. This is also related to my another comments. Directly embedding the whole Principal entity may expose too much internal state.

@adutra adutra Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the new logic here mandate the property PRINCIPAL_ENTITY_ATTRIBUTE_KEY, otherwise, rotation will skip. I'm not sure the new behavior is correct.

I think that's correct. If the principal entity does not exist, it's because the principal is fully external. In that case, Polaris cannot know if the credentials need rotation. It's the external IDP's responsibility to deny the token request from such a user.

@dimas-b dimas-b Jul 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Polaris should not interfere with token generation for external principals.

return principal
.getAttribute(PolarisPrincipal.PRINCIPAL_ENTITY_ATTRIBUTE_KEY, PrincipalEntity.class)
.map(PrincipalEntity::getInternalPropertiesAsMap)
.map(
map ->
map.containsKey(
PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE))
.orElse(false);
}
}
Loading