roryqi commented on issue #10404: URL: https://github.com/apache/gravitino/issues/10404#issuecomment-4311148743
> Building on [PR #10848](https://github.com/apache/gravitino/pull/10848) (which adds group support in setOwner/getOwner), next I'd like to implement the authorizer-side enforcement so isOwner() recognizes group owners. > > **The problem:** loadOwnerPolicy() only processes UserEntity owners. If a GroupEntity is the owner, the ownerRel cache is never populated and isOwner() always returns false. > > **Proposed fix:** The ownerRel cache currently stores `metadataId → Optional<userId>`. I'd change it to store an `OwnerInfo (id, type, name)` so it can represent both user and group owners. In `isOwner()`, group ownership is resolved by checking the user's JWT groups (UserPrincipal.getGroups()). > > I plan to store the `group name` in the cache (alongside id and type) because `UserPrincipal.getGroups()` carries `names` from the JWT token, `NOT Gravitino entity IDs`. This avoids a DB lookup on every isOwner() call. First wanted to confirm if this approach acceptable ? > > cc [@roryqi](https://github.com/roryqi) [@yuqi1129](https://github.com/yuqi1129) [@jerryshao](https://github.com/jerryshao) It's acceptable for me. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
