> > 3) I am uncertain if merging the entity's internal and non-internal > property maps is a safe operation.
Good callout, Alex. We should definitely maintain a strict distinction between internalProperties vs properties, and it is certainly not safe just to merge the two. Generally internalProperties should only ever be directly accessible within the Polaris service itself and are normally part of an internal type-specific structure. In some cases, they are plumbed into first-class API-model members via some transformation, but in other cases they are intentionally fully hidden from the API callers. Looks like the method name first got a bit blurry in https://github.com/apache/polaris/pull/2307/changes#diff-121e077709a9a1b0ab15c99fbc0c5fe3b6208339363b61a27e46de3952b8e6ddR88 - it's not clear at a glance whether PolarisPrincipal.getProperties there was intended to preserve the "internal-only" aspect of the properties. In any case, I'm +1 to removing `PolarisPrincipal.getProperties` as well and requiring any callsites that currently need the internal properties to obtain the underlying PrincipalEntity if it exists. On Mon, May 4, 2026 at 11:15 AM Yufei Gu <[email protected]> wrote: > Hi Alex, Selva, > > Thanks Alex, that context is helpful. I agree my earlier read was too > narrow: simply switching PolarisPrincipal to expose the full > PrincipalEntity property map would blur the boundary between the > authenticated principal and the persisted entity, and it may conflict with > the detached-principal direction. > > For PR #4292, I would not merge the direct property-copy approach as-is. If > a component needs the user-defined attributes of the persisted principal > entity, it should either fetch that entity explicitly or use a clearly > modeled security identity attribute, as Alex suggested, rather than making > PolarisPrincipal a carrier for all entity properties. This thread [1] > discussed a way by introducing a container named either > authenticatedMetadata or AuthorizationContext. I think that's the right > direction. > > So I think the next step should be to clarify the intended contract first: > > - PolarisPrincipal should expose only authentication/principal identity > data needed by authn/authz paths, potentially via an object like > AuthorizationContext. > - Arbitrary user-defined PrincipalEntity attributes should remain entity > data, not implicit principal identity data. > > Selva, thanks for opening the issue and PR. The test case and Ranger > integration are still useful because they capture the behavior gap and use > cases, but I think the fix should probably move in the direction Alex > outlined rather than expanding PolarisPrincipal directly. > > 1. https://lists.apache.org/thread/fmy8c43dmr8hmtrhphtcwp8yogqhkwqw > > Yufei > > On Sun, 26 Apr 2026 16:35:18 +0200, Alexandre Dutra [email protected] > wrote: > > Hi Selva, > > I can provide some context on why only internal properties are > currently exposed in PolarisPrincipal. > > Previously, PolarisPrincipal directly surfaced the underlying > PrincipalEntity. However, this direct dependency was removed to enable > alternative authentication mechanisms 1 > <https://github.com/apache/polaris/pull/2307>. > > A specific challenge during this transition was that several > components relied on inspecting the principal’s internal properties. > For example, the legacy code looked like this: > > principal > .getPrincipalEntity() > .getInternalPropertiesAsMap() > > .containsKey(PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE) > > Because the code only required access to internal properties at that > time, we moved the internal properties map directly into > PolarisPrincipal. This simplified the access pattern to: > > principal > .getProperties() > > .containsKey(PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE) > > With that, I have a few concerns regarding your proposal: > > 1) I am not sure that PolarisPrincipal should expose all attributes of > a PrincipalEntity. These are different concepts. If you are writing a > component that needs to access the properties of the entity > corresponding to the authenticated principal, I’d recommend fetching > the entity from the metastore explicitly. > > 2) We should not assume that all PolarisPrincipal instances are backed > by a PrincipalEntity. This may be true today, but the goal is to > achieve “detached” principals one day (principals not persisted in the > metastore). > > 3) I am uncertain if merging the entity’s internal and non-internal > property maps is a safe operation. > > 4) There is an ongoing conversation 2 > <https://lists.apache.org/thread/fmy8c43dmr8hmtrhphtcwp8yogqhkwqw> about > redesigning the > PolarisPrincipal interface. Some contributors feel it already exposes > too much - such as the getToken() method. The preferred direction may > be to further isolate the principal from credentials and other > attributes. Adding more attributes now might conflict with that goal. > > For example, a cleaner approach in my opinion could be for > authenticators to produce a Quarkus security identity with the > following shape: > > SecurityIdentity: > - principal: PolarisPrincipal > - credentials: > - PolarisCredential (principal ID, name and roles) > - TokenCredential (the OAuth2 access token if available) > - etc. > - attributes: > - POLARIS_ENTITY => PolarisEntity (optional) > > Then you would be able to inject the security identity and get the > entire principal entity from its attributes. > > Thanks, > Alex > > On Sat, Apr 25, 2026 at 6:55 AM Selvamohan Neethiraj > [email protected] wrote: > > Hi Yufei, > > Thank you for confirming that this is likely a bug and for pointing to the > relevant logic and PR reference. > > I have created Issue #4291 to track this: > https://github.com/apache/polaris/issues/4291 > > I have also submitted a proposed fix along with test coverage in PR #4292: > https://github.com/apache/polaris/pull/4292 > > Could you please review the PR and share your feedback? > > Thanks again for your guidance. > > Regards, > Selva- > > On Apr 24, 2026, at 7:44 PM, Yufei Gu [email protected] wrote: > > Hi Selva, > > It is likely a bug. The logic 1 > <https://github.com/apache/polaris/pull/2307> was introduced in PR 23072 > <https://lists.apache.org/thread/fmy8c43dmr8hmtrhphtcwp8yogqhkwqw>, before > that, getProperties() did not exist. I think using > principalEntity.getPropertiesAsMap() makes more sense in 1 > <https://github.com/apache/polaris/pull/2307>. > > 1. > > https://github-personal/flyrain/polaris/blob/4d90f53f2d360e622f0d6e3006dedcec497b1d38/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisPrincipal.java#L46 > 2. https://github.com/apache/polaris/pull/2307 > > Yufei > > On Thu, Apr 23, 2026 at 6:10 PM Selvamohan Neethiraj [email protected] > wrote: > > Following up on my earlier email, I was able to trace the issue and am now > trying to understand the reasoning behind the current implementation. > > When a PolarisPrincipal (org.apache.polaris.core.auth.PolarisPrincipal) is > created from a PrincipalEntity > (org.apache.polaris.core.entity.PrincipalEntity), it appears to copy only > the internal properties using getInternalPropertiesAsMap(). This preserves > attributes such as clientId, but drops user-defined attributes. > > Based on this behavior, it seems that using > principalEntity.getPropertiesAsMap() instead of > principalEntity.getInternalPropertiesAsMap() would retain both internal and > user-defined attributes. > > Is there a specific reason why user-defined attributes are intentionally > excluded when creating a PolarisPrincipal object? > > Regards, > Selva- > > On Apr 23, 2026, at 1:34 PM, Selvamohan Neethiraj [email protected] > wrote: > > Hi, > > I am using the REST API /api/management/v1/principals to create a new > principal with user attributes (for example: region=northamerica). The API > call completes successfully, and the response correctly includes the > specified user attributes. > > However, when I use the returned client-id and client-secret to obtain > an OAuth token from /api/catalog/v1/oauth/tokens, and then use that token > to perform other API operations (such as listing catalogs via > /api/management/v1/catalogs), the server-side Polaris principal does not > appear to include the user attributes. > > Specifically, the user attributes defined during principal creation do > not seem to be available during subsequent API calls authenticated using > the generated OAuth token. > > Could you please confirm: > > 1. Whether this is the expected behavior, or > 2. If there is an additional step required to propagate or include > principal attributes when generating or using OAuth tokens, or > 3. If this might be a bug. > > Thanks in advance for your guidance. > > Best regards, > Selva >
