roryqi commented on code in PR #12850:
URL: https://github.com/apache/gravitino/pull/12850#discussion_r3930418319
##########
lineage/src/main/java/org/apache/gravitino/lineage/source/rest/LineageOperations.java:
##########
@@ -53,16 +56,25 @@ public LineageOperations(LineageDispatcher
lineageDispatcher) {
@Produces(MediaType.APPLICATION_JSON)
@Timed(name = "post-lineage." + MetricNames.HTTP_PROCESS_DURATION, absolute
= true)
@ResponseMetered(name = "post-lineage", absolute = true)
- public Response postLineage(OpenLineage.RunEvent event) {
- LOG.info(
- "Open lineage event, run id:{}, job name:{}",
- org.apache.gravitino.lineage.Utils.getRunID(event),
- org.apache.gravitino.lineage.Utils.getJobName(event));
+ @AuthorizationExpression(expression =
AuthorizationExpressionConstants.CAN_ACCESS_METADATA)
Review Comment:
Addressed in b874aa471. Metadata visibility remains intentional for both
inputs and outputs because this endpoint records producer-reported lineage
rather than attesting that the caller performed the underlying read or write.
The OpenAPI description now states that rationale explicitly, and a regression
test pins that a SELECT-only caller may report an output. A stronger
lineage-attestation privilege would be a separate authorization feature.
##########
server/src/main/java/org/apache/gravitino/server/web/filter/authorization/LineageAuthorizationExecutor.java:
##########
@@ -0,0 +1,179 @@
+/*
+ * 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.filter.authorization;
+
+import static
org.apache.gravitino.server.web.filter.ParameterUtil.extractFromParameters;
+
+import com.google.common.base.Preconditions;
+import io.openlineage.server.OpenLineage.Dataset;
+import io.openlineage.server.OpenLineage.DatasetFacet;
+import io.openlineage.server.OpenLineage.DatasetFacets;
+import io.openlineage.server.OpenLineage.RunEvent;
+import java.lang.reflect.Parameter;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.MetadataObjects;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.authorization.AuthorizationRequestContext;
+import org.apache.gravitino.lineage.source.rest.LineageEventValidator;
+import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionEvaluator;
+import org.apache.gravitino.utils.MetadataObjectUtil;
+import org.apache.gravitino.utils.NameIdentifierUtil;
+
+/** Authorization executor for every input and output dataset in an
OpenLineage event. */
+public class LineageAuthorizationExecutor implements AuthorizationExecutor {
+
+ private static final String DATASET_TYPE_FACET = "datasetType";
+
+ private final Parameter[] parameters;
+ private final Object[] args;
+ private final String expression;
+ private List<AuthorizationTarget> authorizationTargets = List.of();
+ private boolean authorizationTargetsResolved;
+
+ /**
+ * Creates an authorization executor for an OpenLineage event.
+ *
+ * @param parameters parameters of the intercepted REST method
+ * @param args arguments passed to the intercepted REST method
+ * @param expression authorization expression to evaluate for every dataset
+ */
+ public LineageAuthorizationExecutor(Parameter[] parameters, Object[] args,
String expression) {
+ this.parameters = parameters;
+ this.args = args;
+ this.expression = expression;
+ }
+
+ @Override
+ public Optional<String> getAuthorizationMetalake() {
+ RunEvent event = extractRunEvent();
+ resolveAuthorizationTargets(event);
+ return Optional.of(event.getJob().getNamespace());
+ }
+
+ @Override
+ public boolean execute(AuthorizationRequestContext context) {
+ if (!authorizationTargetsResolved) {
+ resolveAuthorizationTargets(extractRunEvent());
+ }
+
+ AuthorizationExpressionEvaluator evaluator = new
AuthorizationExpressionEvaluator(expression);
+ context.setOriginalAuthorizationExpression(expression);
+
+ for (AuthorizationTarget target : authorizationTargets) {
+ if (!evaluator.evaluate(
+ target.metadataContext, Map.of(), context,
Optional.of(target.entityType.name()))) {
+ return false;
+ }
+ }
+ return true;
Review Comment:
Addressed in b874aa471. Dataset-less RunEvents remain allowed because inputs
and outputs are optional OpenLineage fields. Such an event still requires
membership in the metalake identified by `job.namespace`; there is simply no
dataset target to authorize. This behavior is now documented and covered by a
regression test.
##########
server/src/main/java/org/apache/gravitino/server/web/filter/authorization/LineageAuthorizationExecutor.java:
##########
@@ -0,0 +1,179 @@
+/*
+ * 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.filter.authorization;
+
+import static
org.apache.gravitino.server.web.filter.ParameterUtil.extractFromParameters;
+
+import com.google.common.base.Preconditions;
+import io.openlineage.server.OpenLineage.Dataset;
+import io.openlineage.server.OpenLineage.DatasetFacet;
+import io.openlineage.server.OpenLineage.DatasetFacets;
+import io.openlineage.server.OpenLineage.RunEvent;
+import java.lang.reflect.Parameter;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.MetadataObjects;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.authorization.AuthorizationRequestContext;
+import org.apache.gravitino.lineage.source.rest.LineageEventValidator;
+import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionEvaluator;
+import org.apache.gravitino.utils.MetadataObjectUtil;
+import org.apache.gravitino.utils.NameIdentifierUtil;
+
+/** Authorization executor for every input and output dataset in an
OpenLineage event. */
+public class LineageAuthorizationExecutor implements AuthorizationExecutor {
+
+ private static final String DATASET_TYPE_FACET = "datasetType";
+
+ private final Parameter[] parameters;
+ private final Object[] args;
+ private final String expression;
+ private List<AuthorizationTarget> authorizationTargets = List.of();
+ private boolean authorizationTargetsResolved;
+
+ /**
+ * Creates an authorization executor for an OpenLineage event.
+ *
+ * @param parameters parameters of the intercepted REST method
+ * @param args arguments passed to the intercepted REST method
+ * @param expression authorization expression to evaluate for every dataset
+ */
+ public LineageAuthorizationExecutor(Parameter[] parameters, Object[] args,
String expression) {
+ this.parameters = parameters;
+ this.args = args;
+ this.expression = expression;
+ }
+
+ @Override
+ public Optional<String> getAuthorizationMetalake() {
+ RunEvent event = extractRunEvent();
+ resolveAuthorizationTargets(event);
+ return Optional.of(event.getJob().getNamespace());
+ }
+
+ @Override
+ public boolean execute(AuthorizationRequestContext context) {
+ if (!authorizationTargetsResolved) {
+ resolveAuthorizationTargets(extractRunEvent());
+ }
+
+ AuthorizationExpressionEvaluator evaluator = new
AuthorizationExpressionEvaluator(expression);
+ context.setOriginalAuthorizationExpression(expression);
+
+ for (AuthorizationTarget target : authorizationTargets) {
+ if (!evaluator.evaluate(
+ target.metadataContext, Map.of(), context,
Optional.of(target.entityType.name()))) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ static MetadataObject.Type getMetadataType(Dataset dataset) {
+ DatasetFacets facets = dataset.getFacets();
+ if (facets == null) {
+ return MetadataObject.Type.TABLE;
+ }
+
+ DatasetFacet datasetTypeFacet =
facets.getAdditionalProperties().get(DATASET_TYPE_FACET);
+ if (datasetTypeFacet == null) {
+ return MetadataObject.Type.TABLE;
+ }
+
+ Object datasetType =
datasetTypeFacet.getAdditionalProperties().get(DATASET_TYPE_FACET);
+ Preconditions.checkArgument(
+ datasetType instanceof String && StringUtils.isNotBlank((String)
datasetType),
+ "The datasetType facet must contain a non-blank datasetType");
+
+ return switch (((String) datasetType).toUpperCase(Locale.ROOT)) {
+ case "TABLE" -> MetadataObject.Type.TABLE;
+ case "VIEW" -> MetadataObject.Type.VIEW;
+ case "FILE", "FILESET" -> MetadataObject.Type.FILESET;
+ case "MODEL", "MODEL_VERSION" -> MetadataObject.Type.MODEL;
Review Comment:
Fixed in b874aa471. I dropped `MODEL_VERSION` from the supported
`datasetType` values rather than accepting a misleading three-part identifier.
The OpenAPI contract was updated, the prior three-part MODEL_VERSION test was
removed, and MODEL_VERSION is now explicitly covered as an unsupported type
returning 400.
##########
server/src/main/java/org/apache/gravitino/server/web/filter/authorization/LineageAuthorizationExecutor.java:
##########
@@ -0,0 +1,179 @@
+/*
+ * 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.filter.authorization;
+
+import static
org.apache.gravitino.server.web.filter.ParameterUtil.extractFromParameters;
+
+import com.google.common.base.Preconditions;
+import io.openlineage.server.OpenLineage.Dataset;
+import io.openlineage.server.OpenLineage.DatasetFacet;
+import io.openlineage.server.OpenLineage.DatasetFacets;
+import io.openlineage.server.OpenLineage.RunEvent;
+import java.lang.reflect.Parameter;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.MetadataObjects;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.authorization.AuthorizationRequestContext;
+import org.apache.gravitino.lineage.source.rest.LineageEventValidator;
+import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionEvaluator;
+import org.apache.gravitino.utils.MetadataObjectUtil;
+import org.apache.gravitino.utils.NameIdentifierUtil;
+
+/** Authorization executor for every input and output dataset in an
OpenLineage event. */
+public class LineageAuthorizationExecutor implements AuthorizationExecutor {
+
+ private static final String DATASET_TYPE_FACET = "datasetType";
+
+ private final Parameter[] parameters;
+ private final Object[] args;
+ private final String expression;
+ private List<AuthorizationTarget> authorizationTargets = List.of();
+ private boolean authorizationTargetsResolved;
+
+ /**
+ * Creates an authorization executor for an OpenLineage event.
+ *
+ * @param parameters parameters of the intercepted REST method
+ * @param args arguments passed to the intercepted REST method
+ * @param expression authorization expression to evaluate for every dataset
+ */
+ public LineageAuthorizationExecutor(Parameter[] parameters, Object[] args,
String expression) {
+ this.parameters = parameters;
+ this.args = args;
+ this.expression = expression;
+ }
+
+ @Override
+ public Optional<String> getAuthorizationMetalake() {
Review Comment:
Fixed in b874aa471. `getAuthorizationMetalake()` now only extracts
`job.namespace` and validates the fields required to do so. Full event
validation and authorization-target resolution are deferred to `execute()`,
with target-preparation failures still mapped specifically to 400 while
authorizer failures remain 500.
##########
server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java:
##########
@@ -238,6 +193,40 @@ public Object invoke(MethodInvocation methodInvocation)
throws Throwable {
secondaryExpression,
secondaryExpressionCondition,
expressionAnnotation.allowCheckExistence());
+ try {
+ Optional<String> dynamicMetalake =
executor.getAuthorizationMetalake();
+ if (dynamicMetalake.isPresent()
+ && authorizationMetalake.isPresent()
+ &&
!dynamicMetalake.get().equals(authorizationMetalake.get())) {
+ throw new IllegalArgumentException(
+ String.format(
+ "Authorization request metalake '%s' does not match
path metalake '%s'",
+ dynamicMetalake.get(), authorizationMetalake.get()));
+ }
+ if (dynamicMetalake.isPresent()) {
+ authorizationMetalake = dynamicMetalake;
+ }
+ } catch (IllegalArgumentException exception) {
+ LOG.warn("Invalid authorization request", exception);
+ return Utils.illegalArguments(exception.getMessage(), exception);
+ }
+ }
+
+ if (authorizationMetalake.isPresent()) {
+ Optional<Response> validationFailure =
+ validateCurrentUserAndActiveRoles(
Review Comment:
Fixed in b874aa471. Path-derived metalakes are now validated before executor
construction, preserving the existing ordering for all current REST endpoints.
Only a dynamically resolved metalake is validated after executor creation. A
regression test verifies that a non-member receives 403 before a malformed
create-schema request can be inspected.
##########
server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java:
##########
@@ -267,6 +256,65 @@ public Object invoke(MethodInvocation methodInvocation)
throws Throwable {
}
}
+ private Optional<Response> validateCurrentUserAndActiveRoles(
+ NameIdentifier metalakeIdent,
+ AuthorizationRequestContext authorizationRequestContext,
+ AuthorizationExpression expressionAnnotation,
+ Map<Entity.EntityType, NameIdentifier> metadataContext,
+ Method method,
+ String expression) {
+ String currentUser = PrincipalUtils.getCurrentUserName();
+ try {
+ AuthorizationUtils.checkCurrentUser(
+ metalakeIdent.name(), currentUser, authorizationRequestContext);
+ } catch (NoSuchMetalakeException e) {
Review Comment:
Fixed in b874aa471. A nonexistent dynamically resolved metalake now returns
400 with a message that identifies `job.namespace`. The existing 403 behavior
for nonexistent path-derived metalakes remains unchanged. Regression coverage
verifies the dynamic lineage response and message.
--
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]
