sungwy commented on PR #3228: URL: https://github.com/apache/polaris/pull/3228#issuecomment-3647641636
Hi @snazy - I took a look at this PR again after yesterday's discussion, and I now fully understand your point 😃 I'm completely in agreement with your approach. I think the question of whether the additional `ResolvedPolarisEntity` grant information needs to be fetched and used in a authorization request should be coupled together with the mode of authorization (`PolarisAuthorizer`) instead of relying on a separate parameter. I am curious to hear your thoughts on the following two questions: 1. Do we think there's a need to introducing three separate flags for `requiresPrincipalRoles`, `requiresCatalogRoles`, `requiresResolvedEntities`, as opposed to having a single one like: `requiresResolvedPolarisEntity` - in the existing use cases, they seem to be coupled together 2. What are your thoughts on an alternate approach of pushing the `Supplier`s of these entities into the `PolarisAuthorizer` for lazy evaluation instead, as opposed to exposing public boolean flags? This way, we'd remove a conditional evaluation within the Handlers, and instead push the responsibility of deciding what context is needed and fetching it only into the `PolarisAuthorizer`. It'll be a more complicated refactoring than what's proposed in the PR, but I wanted to hear your thoughts on 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
