Copilot commented on code in PR #13512:
URL: https://github.com/apache/gravitino/pull/13512#discussion_r4163092806
##########
server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java:
##########
@@ -109,6 +110,7 @@ public Filter getDescriptorFilter() {
ViewOperations.class.getName(),
ModelOperations.class.getName(),
FunctionOperations.class.getName(),
+ SemanticModelOperations.class.getName(),
Review Comment:
Registering this resource does not authorize all of its endpoints: the
interceptor skips methods without `@AuthorizationExpression`, while
`listSemanticModels`, `alterSemanticModel`, and `dropSemanticModel` remain
unannotated. As a result, listing returns every model name and callers can
alter or drop models without the Semantic Model privileges defined for those
operations. Add the schema gate plus
`FILTER_SEMANTIC_MODEL_AUTHORIZATION_EXPRESSION` filtering for list, and apply
the existing modify/drop expressions and metadata annotations to alter/drop,
with denied-path tests.
##########
server/src/test/java/org/apache/gravitino/server/web/rest/TestSemanticModelSourceValidator.java:
##########
@@ -0,0 +1,247 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.server.web.rest;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
+
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.authorization.GravitinoAuthorizer;
+import org.apache.gravitino.authorization.Privilege;
+import org.apache.gravitino.catalog.TableDispatcher;
+import org.apache.gravitino.catalog.ViewDispatcher;
+import org.apache.gravitino.exceptions.ConnectionFailedException;
+import org.apache.gravitino.exceptions.ForbiddenException;
+import org.apache.gravitino.exceptions.IllegalSemanticModelException;
+import org.apache.gravitino.exceptions.NoSuchTableException;
+import org.apache.gravitino.exceptions.NoSuchViewException;
+import org.apache.gravitino.rel.Column;
+import org.apache.gravitino.rel.Table;
+import org.apache.gravitino.rel.View;
+import org.apache.gravitino.semantic.Dataset;
+import org.apache.gravitino.semantic.Relationship;
+import org.apache.gravitino.semantic.SemanticModelDefinition;
+import org.apache.gravitino.server.authorization.PassThroughAuthorizer;
+import org.junit.jupiter.api.Test;
+
+/** Tests catalog-backed validation independently of model persistence. */
+public class TestSemanticModelSourceValidator {
+ private final TableDispatcher tables = mock(TableDispatcher.class);
+ private final ViewDispatcher views = mock(ViewDispatcher.class);
+ private GravitinoAuthorizer authorizer = new PassThroughAuthorizer();
+ private final SemanticModelSourceValidator validator =
+ new SemanticModelSourceValidator(tables, views, () -> authorizer);
+ private static final NameIdentifier SOURCE =
+ NameIdentifier.of("other_catalog", "schema", "orders");
+ private static final NameIdentifier FULL_SOURCE =
+ NameIdentifier.of("metalake", "other_catalog", "schema", "orders");
Review Comment:
Move these static constants before the instance fields. The repository's
Java member-ordering convention places static constants first, followed by
instance fields.
--
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]
