imbajin commented on code in PR #3159:
URL: https://github.com/apache/hugegraph/pull/3159#discussion_r3883993667


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java:
##########
@@ -203,20 +205,25 @@ public String checkDefaultRole(@Context GraphManager 
manager,
             defaultRole.equals(HugeDefaultRole.SPACE)) {
             throw new ForbiddenException("Forbidden to check role " + role);
         }
-        boolean hasGraph = defaultRole.equals(HugeDefaultRole.OBSERVER);
-        E.checkArgument(!hasGraph || StringUtils.isNotEmpty(graph),
-                        "Must set a graph for observer");
+        boolean hasGraph = defaultRole.equals(HugeDefaultRole.OBSERVER) &&
+                           StringUtils.isNotEmpty(graph);
         if (hasGraph) {
             validGraph(manager, name, graph);
         }
 
         boolean result;
         if (hasGraph) {
-            result = authManager.isDefaultRole(name, graph, user,
-                                               defaultRole);
+            result = authManager.isDefaultRole(name, graph, user, defaultRole);

Review Comment:
   Fixed on `6851b1cd`: concrete-graph checks now recognize the explicit 
`ALL_GRAPHS` role in `GraphSpaceAPI`, `ManagerAPI`, and 
`StandardAuthManagerV2`. Focused endpoint coverage is included.



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeAuthenticator.java:
##########
@@ -359,8 +363,15 @@ public static boolean match(Object role, RolePermission 
grant,
                 }
             }
 
-            RolePermission rolePerm = RolePermission.fromJson(role);
-            return rolePerm.contains(grant);
+            RolePermission grantedRole = RolePermission.fromJson(grant);
+            RolePerm rolePerm = RolePerm.fromJson(role);
+            if (resourceObject != null &&

Review Comment:
   Fixed on `6851b1cd`: the GraphSpace-manager shortcut now accepts only a 
single-space grant and rejects mixed-space role disclosure. Cross-space 
regressions are covered.



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java:
##########
@@ -2461,4 +2518,36 @@ public String toString() {
             return this.origin.toString();
         }
     }
+
+    private static Set<HugePermission> traversalPermissions(
+                                       Traversal.Admin<?, ?> traversal) {
+        Set<HugePermission> permissions = EnumSet.noneOf(HugePermission.class);
+        collectTraversalPermissions(traversal, permissions);
+        return permissions;
+    }
+
+    private static void collectTraversalPermissions(
+                        Traversal.Admin<?, ?> traversal,
+                        Set<HugePermission> permissions) {
+        for (Step<?, ?> step : traversal.getSteps()) {
+            if (step instanceof AddVertexStartStep ||

Review Comment:
   Removed on `6851b1cd`: the incomplete traversal mutation classifier and 
pre-check are gone. This PR keeps the established GREMLIN EXECUTE contract 
instead of introducing a partial WRITE/DELETE model.



##########
.github/workflows/docker-build-ci.yml:
##########
@@ -24,10 +24,17 @@ on:
       - 'release-*'
   pull_request:
     paths:
-      - '**/Dockerfile*'
+      - '.github/workflows/docker-build-ci.yml'
       - '.dockerignore'
-      - 'hugegraph-server/hugegraph-dist/docker/**'
-      - 'hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh'
+      - '.mvn/**'
+      - 'pom.xml'
+      - 'hugegraph-commons/**'

Review Comment:
   Fixed on `6851b1cd`: `docker/hbase/**` is restored to the existing Docker 
build path filter without adding a separate workflow.



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -2434,19 +2428,14 @@ public static ConsumerWrapper wrap(Consumer consumer) {
 
         @Override
         public void accept(T t) {
-            boolean grpcThread = false;
             try {
-                grpcThread = Thread.currentThread().getName().contains("grpc");
-                if (grpcThread) {
-                    HugeGraphAuthProxy.setAdmin();
+                if (Thread.currentThread().getName().contains("grpc")) {

Review Comment:
   Fixed on `6851b1cd`: the internal admin callback saves, clears, and restores 
`AuthContext`, proxy contexts, request GraphSpace, and `TaskManager` context, 
including the exceptional path. Focused tests cover restoration.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to