laserninja commented on code in PR #12867:
URL: https://github.com/apache/gravitino/pull/12867#discussion_r4225589756
##########
core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java:
##########
@@ -473,6 +501,65 @@ public static Role filterSecurableObjects(
.build();
}
+ // Compare the connector-visible states before choosing the callback and its
payload. A
+ // Semantic Model-only change must neither contact a connector nor leak into
its role snapshot.
+ static void notifyRolePrivilegesUpdated(
+ String metalake,
+ RoleEntity role,
+ @Nullable SecurableObject before,
+ @Nullable SecurableObject after) {
+ SecurableObject previous = connectorSecurableObject(before);
+ SecurableObject next = connectorSecurableObject(after);
+ if (previous == null && next == null) {
+ return;
+ }
+ RoleChange change;
+ if (previous == null) {
+ change = RoleChange.addSecurableObject(role.name(), next);
+ } else if (next == null) {
+ change = RoleChange.removeSecurableObject(role.name(), previous);
+ } else {
+ if
(Sets.newHashSet(previous.privileges()).equals(Sets.newHashSet(next.privileges())))
{
+ return;
+ }
+ change = RoleChange.updateSecurableObject(role.name(), previous, next);
+ }
+ // Include the previous privileges when selecting catalogs, so removing
the last inherited
+ // connector privilege still reaches the plugin that received it.
+ SecurableObject target = next == null ? previous : next;
+ Set<Privilege> affectedPrivileges = Sets.newHashSet(target.privileges());
+ if (previous != null) {
+ affectedPrivileges.addAll(previous.privileges());
+ }
+ callAuthorizationPluginForSecurableObjects(
+ metalake,
+ List.of(
+ SecurableObjects.parse(
+ target.fullName(), target.type(),
Lists.newArrayList(affectedPrivileges))),
+ (plugin, catalog) ->
+ plugin.onRoleUpdated(filterSecurableObjects(role, metalake,
catalog), change));
+ }
+
+ @Nullable
+ private static SecurableObject connectorSecurableObject(@Nullable
SecurableObject object) {
+ if (object == null) {
+ return null;
+ }
+ List<Privilege> privileges = connectorPrivileges(object);
+ return privileges.isEmpty()
+ ? null
+ : SecurableObjects.parse(object.fullName(), object.type(), privileges);
+ }
+
+ private static List<Privilege> connectorPrivileges(SecurableObject object) {
Review Comment:
Addressed in #13718. Added `SKIP_APPLY_PRIVILEGES` and used it for both
connector callback selection and privilege filtering. Catalog-type validation
still uses `SEMANTIC_MODEL_PRIVILEGES`, so connector exclusions now have a
separate constant that can grow independently. Tests cover new and legacy
semantic-model privilege names in semantic-only and mixed grants.
##########
api/src/main/java/org/apache/gravitino/authorization/Privilege.java:
##########
@@ -171,7 +171,13 @@ enum Name {
* credential secrets in the {@code getSecrets} result. Alone it does not
authorize {@code
* getSecrets}. Does not authorize {@code getCredentials}.
*/
- INCLUDE_CREDENTIAL_SECRETS(0L, 1L << 38);
+ INCLUDE_CREDENTIAL_SECRETS(0L, 1L << 38),
+ /** The privilege to create a semantic model. */
+ CREATE_SEMANTIC_MODEL(0L, 1L << 39),
+ /** The privilege to discover a semantic model and load its definition. */
+ SELECT_SEMANTIC_MODEL(0L, 1L << 40),
Review Comment:
Implemented in #13718. `USE_SEMANTIC_MODEL` is now the preferred API and
documented name, with the authorization expressions updated accordingly. The
existing permission bit is unchanged, and `SELECT_SEMANTIC_MODEL` plus its Java
helper remain deprecated aliases so stored grants and existing callers continue
to work. Added coverage for revocation through either name. All 83 focused
tests, formatting, OpenAPI validation, and Javadoc builds passed; Docker
integration suites were not run.
--
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]
