diff --git a/.editorconfig b/.editorconfig index 04a6e64a9f..c64c7bae2b 100644 --- a/.editorconfig +++ b/.editorconfig @@ -21,9 +21,9 @@ root = true charset = utf-8 end_of_line = lf insert_final_newline = true -max_line_length = 100 +max_line_length = 120 ij_wrap_on_typing = true -ij_visual_guides = 100 +ij_visual_guides = 120 [*.{java,xml,py}] diff --git a/.github/workflows/check-dependencies.yml b/.github/workflows/check-dependencies.yml index fa804e260c..447162d67f 100644 --- a/.github/workflows/check-dependencies.yml +++ b/.github/workflows/check-dependencies.yml @@ -47,7 +47,7 @@ jobs: - name: 'Checkout Repository' uses: actions/checkout@v4 - name: 'Dependency Review' - uses: actions/dependency-review-action@v3 + uses: actions/dependency-review-action@v5 # Refer: https://github.com/actions/dependency-review-action with: # TODO: reset critical to low before releasing diff --git a/.github/workflows/docker-build-ci.yml b/.github/workflows/docker-build-ci.yml index ada012be80..b1be078d91 100644 --- a/.github/workflows/docker-build-ci.yml +++ b/.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/**' + - 'hugegraph-cluster-test/**' + - 'hugegraph-pd/**' + - 'hugegraph-store/**' + - 'hugegraph-struct/**' + - 'hugegraph-server/**' + - 'install-dist/**' jobs: docker-build: @@ -47,7 +54,8 @@ jobs: - name: Build ${{ matrix.dockerfile }} run: | - IMAGE_ID=$(docker build -q -f ${{ matrix.dockerfile }} .) + IMAGE_ID=$(docker build -q --build-arg SOURCE_REVISION="$GITHUB_SHA" \ + -f ${{ matrix.dockerfile }} .) echo "Built: $IMAGE_ID" echo "IMAGE_ID=$IMAGE_ID" >> "$GITHUB_ENV" HC=$(docker inspect --format='{{json .Config.Healthcheck}}' "$IMAGE_ID") @@ -78,3 +86,34 @@ jobs: echo "ERROR: no usable socket-table tool (ss/netstat) in ${{ matrix.dockerfile }}" exit 1 } + + - name: Server image API versions match source + if: ${{ startsWith(matrix.dockerfile, 'hugegraph-server/') }} + run: | + CHECK_DIR=$(mktemp -d) + trap 'rm -rf "$CHECK_DIR"' EXIT + docker run --rm --entrypoint bash \ + -v "$CHECK_DIR:/check" "$IMAGE_ID" -c \ + 'cp /hugegraph-server/lib/hugegraph-api-*.jar \ + /hugegraph-server/lib/hugegraph-common-*.jar /check/' + + API_JAR=$(find "$CHECK_DIR" -name 'hugegraph-api-*.jar' -print -quit) + COMMON_JAR=$(find "$CHECK_DIR" -name 'hugegraph-common-*.jar' -print -quit) + EXPECTED_MANIFEST=$(sed -n \ + 's|.*\([^<]*\).*|\1|p' \ + hugegraph-server/hugegraph-api/pom.xml) + ACTUAL_MANIFEST=$(unzip -p "$API_JAR" META-INF/MANIFEST.MF | + sed -n 's/^Implementation-Version: *//p' | tr -d '\r') + EXPECTED_PROPERTY=$(sed -n 's/^ApiVersion=//p' \ + hugegraph-commons/hugegraph-common/src/main/resources/version.properties) + ACTUAL_PROPERTY=$(unzip -p "$COMMON_JAR" version.properties | + sed -n 's/^ApiVersion=//p' | tr -d '\r') + + [[ "$ACTUAL_MANIFEST" == "$EXPECTED_MANIFEST" ]] || { + echo "ERROR: API manifest is $ACTUAL_MANIFEST; expected $EXPECTED_MANIFEST" + exit 1 + } + [[ "$ACTUAL_PROPERTY" == "$EXPECTED_PROPERTY" ]] || { + echo "ERROR: API property is $ACTUAL_PROPERTY; expected $EXPECTED_PROPERTY" + exit 1 + } diff --git a/.serena/memories/code_style_and_conventions.md b/.serena/memories/code_style_and_conventions.md index 159920cd3b..7a4c310e0b 100644 --- a/.serena/memories/code_style_and_conventions.md +++ b/.serena/memories/code_style_and_conventions.md @@ -6,7 +6,7 @@ - `.licenserc.yaml` + apache-rat-plugin + skywalking-eyes — License header validation ## Core Rules -- **Line length**: 100 chars (120 for XML) +- **Line length**: 120 chars - **Indent**: 4 spaces, continuation 8 spaces - **Charset**: UTF-8, LF line endings, final newline - **Imports**: Sorted `$*` → `java` → `javax` → `org` → `com` → `*`, no star imports (threshold 100) diff --git a/AGENTS.md b/AGENTS.md index 2d6e81b15b..07daf17662 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -85,7 +85,7 @@ Before writing new tests, check existing suites under `hugegraph-server/hugegrap ## Style & Pre-commit -- Line 100, 4-space indent, LF, UTF-8, **no star imports** +- Line 120, 4-space indent, LF, UTF-8, **no star imports** - Commit format: `feat|fix|refactor(module): msg` - Run before pushing: ```bash diff --git a/README.md b/README.md index adf9792776..f4543e073f 100644 --- a/README.md +++ b/README.md @@ -342,7 +342,7 @@ For detailed architecture and development guidance, see [AGENTS.md](AGENTS.md). - Try modifying a test and see what breaks 5. **Code Standards** - - Line length: 100 characters + - Line length: 120 characters - Indentation: 4 spaces - No star imports - Commit format: `feat|fix|refactor(module): description` diff --git a/hugegraph-commons/hugegraph-common/src/main/resources/version.properties b/hugegraph-commons/hugegraph-common/src/main/resources/version.properties index 2dffc6f3a6..8d48ef39ca 100644 --- a/hugegraph-commons/hugegraph-common/src/main/resources/version.properties +++ b/hugegraph-commons/hugegraph-common/src/main/resources/version.properties @@ -17,7 +17,7 @@ # hugegraph-common follows the project version defined by ${revision} in the root pom.xml, # and VersionInBash needs to be updated in this file. Version=${revision} -ApiVersion=0.71 +ApiVersion=0.72 ApiCheckBeginVersion=1.0 ApiCheckEndVersion=2.0 VersionInBash=1.7.0 diff --git a/hugegraph-pd/docs/development.md b/hugegraph-pd/docs/development.md index 3f01b902ea..514bd989a1 100644 --- a/hugegraph-pd/docs/development.md +++ b/hugegraph-pd/docs/development.md @@ -282,7 +282,7 @@ HugeGraph PD follows Apache HugeGraph code style. **Key Style Rules**: - **Indentation**: 4 spaces (no tabs) -- **Line length**: 100 characters (Java), 120 characters (comments) +- **Line length**: 120 characters - **Braces**: K&R style (opening brace on same line) - **Imports**: No wildcard imports (`import java.util.*`) diff --git a/hugegraph-server/Dockerfile b/hugegraph-server/Dockerfile index 5caadd23cb..44bc9aa515 100644 --- a/hugegraph-server/Dockerfile +++ b/hugegraph-server/Dockerfile @@ -25,9 +25,10 @@ WORKDIR /pkg COPY . . ARG MAVEN_ARGS +ARG SOURCE_REVISION=local -RUN --mount=type=cache,target=/root/.m2 \ - mvn package $MAVEN_ARGS -e -B -ntp -Dmaven.test.skip=true -Dmaven.javadoc.skip=true \ +RUN --mount=type=cache,id=hugegraph-maven-${SOURCE_REVISION},target=/root/.m2,sharing=locked \ + mvn install $MAVEN_ARGS -e -B -ntp -Dmaven.test.skip=true -Dmaven.javadoc.skip=true \ && rm ./hugegraph-server/*.tar.gz ./hugegraph-pd/*.tar.gz ./hugegraph-store/*.tar.gz # 2nd stage: runtime env diff --git a/hugegraph-server/Dockerfile-hstore b/hugegraph-server/Dockerfile-hstore index 7cd64e8f3b..5d4d96d773 100644 --- a/hugegraph-server/Dockerfile-hstore +++ b/hugegraph-server/Dockerfile-hstore @@ -25,9 +25,10 @@ WORKDIR /pkg COPY . . ARG MAVEN_ARGS +ARG SOURCE_REVISION=local -RUN --mount=type=cache,target=/root/.m2 \ - mvn package $MAVEN_ARGS -e -B -ntp -DskipTests -Dmaven.javadoc.skip=true \ +RUN --mount=type=cache,id=hugegraph-maven-${SOURCE_REVISION},target=/root/.m2,sharing=locked \ + mvn install $MAVEN_ARGS -e -B -ntp -DskipTests -Dmaven.javadoc.skip=true \ && rm ./hugegraph-server/*.tar.gz ./hugegraph-pd/*.tar.gz ./hugegraph-store/*.tar.gz # 2nd stage: runtime env diff --git a/hugegraph-server/hugegraph-api/pom.xml b/hugegraph-server/hugegraph-api/pom.xml index f1a8b918bd..e5a81be208 100644 --- a/hugegraph-server/hugegraph-api/pom.xml +++ b/hugegraph-server/hugegraph-api/pom.xml @@ -201,8 +201,8 @@ - - 0.71.0.0 + + 0.72.0.0 diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/auth/ManagerAPI.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/auth/ManagerAPI.java index 37aee8c657..7f264027ab 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/auth/ManagerAPI.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/auth/ManagerAPI.java @@ -287,20 +287,24 @@ public String checkDefaultRole(@Context GraphManager manager, defaultRole = null; // unreachable, satisfies compiler } validGraphSpace(manager, graphSpace); - 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, graphSpace, graph); } boolean result; if (hasGraph) { - result = authManager.isDefaultRole(graphSpace, graph, user, - defaultRole); + result = authManager.isDefaultRole(graphSpace, graph, user, defaultRole); } else { - result = authManager.isDefaultRole(graphSpace, user, - defaultRole); + result = authManager.isDefaultRole(graphSpace, user, defaultRole); + if (!result && defaultRole.equals(HugeDefaultRole.OBSERVER)) { + for (String currentGraph : manager.graphs(graphSpace)) { + if (authManager.isDefaultRole(graphSpace, currentGraph, user, defaultRole)) { + result = true; + break; + } + } + } } return manager.serializer().writeMap(ImmutableMap.of("check", result)); } diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java index 81f13cf3f0..1aafd5f28f 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java @@ -146,10 +146,7 @@ public String setDefaultRole(@Context GraphManager manager, throw new ForbiddenException("Forbidden to set role " + role.toString()); } - boolean hasGraph = role.equals(HugeDefaultRole.OBSERVER); - - E.checkArgument(!hasGraph || StringUtils.isNotEmpty(graph), - "Must set a graph for observer"); + boolean hasGraph = role.equals(HugeDefaultRole.OBSERVER) && StringUtils.isNotEmpty(graph); if (hasGraph) { validGraph(manager, name, graph); } @@ -164,6 +161,11 @@ public String setDefaultRole(@Context GraphManager manager, result.put("graph", graph); } else { authManager.createSpaceDefaultRole(name, user, role); + if (role.equals(HugeDefaultRole.OBSERVER)) { + for (String currentGraph : manager.graphs(name)) { + authManager.deleteDefaultRole(name, user, role, currentGraph); + } + } } return manager.serializer().writeMap(result); @@ -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); } else { - result = authManager.isDefaultRole(name, user, - defaultRole); + result = authManager.isDefaultRole(name, user, defaultRole); + if (!result && defaultRole.equals(HugeDefaultRole.OBSERVER)) { + for (String currentGraph : manager.graphs(name)) { + if (authManager.isDefaultRole(name, currentGraph, user, defaultRole)) { + result = true; + break; + } + } + } } return manager.serializer().writeMap(ImmutableMap.of("check", result)); } @@ -259,9 +266,7 @@ public void deleteDefaultRole(@Context GraphManager manager, E.checkArgument(false, "Invalid role value '%s'", role); defaultRole = null; // unreachable, satisfies compiler } - 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); } @@ -269,6 +274,11 @@ public void deleteDefaultRole(@Context GraphManager manager, authManager.deleteDefaultRole(name, user, defaultRole, graph); } else { authManager.deleteDefaultRole(name, user, defaultRole); + if (defaultRole.equals(HugeDefaultRole.OBSERVER)) { + for (String currentGraph : manager.graphs(name)) { + authManager.deleteDefaultRole(name, user, defaultRole, currentGraph); + } + } } } diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/SchemaTemplateAPI.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/SchemaTemplateAPI.java index b2c151687c..cffca156cd 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/SchemaTemplateAPI.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/SchemaTemplateAPI.java @@ -20,10 +20,12 @@ import java.util.Date; import java.util.Objects; import java.util.Set; +import java.util.function.Supplier; import org.apache.commons.lang3.StringUtils; import org.apache.hugegraph.HugeException; import org.apache.hugegraph.api.API; +import org.apache.hugegraph.auth.AuthManager; import org.apache.hugegraph.api.filter.StatusFilter; import org.apache.hugegraph.auth.HugeGraphAuthProxy; import org.apache.hugegraph.core.GraphManager; @@ -134,9 +136,8 @@ public void delete(@Context GraphManager manager, "Schema template '%s' does not exist", name); String username = HugeGraphAuthProxy.username(); - boolean isSpace = manager.authManager() - .isSpaceManager(graphSpace, username); - if (Objects.equals(st.creator(), username) || isSpace) { + if (canManage(manager::authManager, graphSpace, st.creator(), + username)) { manager.dropSchemaTemplate(graphSpace, name); } else { throw new ForbiddenException("No permission to delete schema template"); @@ -165,9 +166,8 @@ public String update(@Context GraphManager manager, } String username = HugeGraphAuthProxy.username(); - boolean isSpace = manager.authManager() - .isSpaceManager(graphSpace, username); - if (Objects.equals(old.creator(), username) || isSpace) { + if (canManage(manager::authManager, graphSpace, old.creator(), + username)) { SchemaTemplate template = jsonSchemaTemplate.build(old); template.creator(old.creator()); template.create(old.create()); @@ -180,6 +180,17 @@ public String update(@Context GraphManager manager, } + private static boolean canManage(Supplier authManagerSupplier, + String graphSpace, String creator, + String username) { + if (Objects.equals(creator, username)) { + return true; + } + AuthManager authManager = authManagerSupplier.get(); + return authManager.isAdminManager(username) || + authManager.isSpaceManager(graphSpace, username); + } + private static class JsonSchemaTemplate implements Checkable { @JsonProperty("name") diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeAuthenticator.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeAuthenticator.java index cef1287b14..4bf0edf086 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeAuthenticator.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeAuthenticator.java @@ -290,6 +290,10 @@ private static Object matchedAction(HugePermission action, } for (Map.Entry e : perms.entrySet()) { HugePermission permission = e.getKey(); + if (permission == HugePermission.SPACE || + permission == HugePermission.SPACE_MEMBER) { + continue; + } // Maybe required = ANY if (action.match(permission) || action.equals(HugePermission.EXECUTE)) { @@ -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 && + !RolePermission.isAdmin(grantedRole) && + grantedRole.roles().containsKey(resourceObject.graphSpace()) && + rolePerm.matchSpace(resourceObject.graphSpace(), "space")) { + return true; + } + return RolePermission.fromJson(role).contains(grantedRole); } @SuppressWarnings({"unchecked", "rawtypes"}) diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java index 4b0aed578f..f3440d0e57 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java @@ -22,6 +22,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Date; +import java.util.EnumSet; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -92,10 +93,18 @@ import org.apache.tinkerpop.gremlin.process.traversal.Bytecode; import org.apache.tinkerpop.gremlin.process.traversal.Bytecode.Instruction; import org.apache.tinkerpop.gremlin.process.traversal.Script; +import org.apache.tinkerpop.gremlin.process.traversal.Step; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategies; import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent; +import org.apache.tinkerpop.gremlin.process.traversal.step.filter.DropStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.AddEdgeStartStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.AddEdgeStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.AddVertexStartStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.AddVertexStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.AddPropertyStep; import org.apache.tinkerpop.gremlin.process.traversal.translator.GroovyTranslator; import org.apache.tinkerpop.gremlin.structure.Edge; import org.apache.tinkerpop.gremlin.structure.Element; @@ -154,15 +163,32 @@ static Context setContext(Context context) { } public static void resetContext() { + AuthContext.resetContext(); CONTEXTS.remove(); REQUEST_GRAPH_SPACE.remove(); } public static void resetSpaceContext() { + AuthContext.resetContext(); CONTEXTS.remove(); REQUEST_GRAPH_SPACE.remove(); } + private void prepareAuditLimiter(UserWithRole user) { + if (user == null || user.role() == null || + HugeAuthenticator.ROLE_NONE.equals(user.role())) { + return; + } + Id userKey = auditLimiterKey(user.username()); + this.auditLimiters.getOrFetch(userKey, id -> { + return RateLimiter.create(this.auditLogMaxRate); + }); + } + + private static Id auditLimiterKey(String username) { + return IdGenerator.of(username); + } + /** * Get the graph space from current request URL path */ @@ -178,13 +204,27 @@ public static void setRequestGraphSpace(String graphSpace) { REQUEST_GRAPH_SPACE.set(graphSpace); } - public static Context setAdmin() { - Context old = getContext(); - AuthContext.useAdmin(); - return old; + public static void runAsAdmin(Runnable runnable) { + String old = AuthContext.getContext(); + try { + AuthContext.setContext(User.ADMIN.toJson()); + runnable.run(); + } finally { + if (old == null) { + AuthContext.resetContext(); + } else { + AuthContext.setContext(old); + } + } } public static Context getContext() { + String internalContext = AuthContext.getContext(); + User internalUser = User.fromJson(internalContext); + if (internalUser != null) { + return new Context(internalUser); + } + // Return task context first String taskContext = TaskManager.getContext(); @@ -1546,7 +1586,8 @@ public Id updateUser(HugeUser updatedUser) { String username = currentUsername(); HugeUser user = this.authManager.getUser(updatedUser.id()); if (!user.name().equals(username)) { - E.checkArgument(HugeAuthenticator.USER_ADMIN.equals(username), + E.checkArgument(HugeAuthenticator.USER_ADMIN.equals(username) || + this.authManager.isAdminManager(username), "Only the user themselves or the admin can change this user", user.name()); this.updateCreator(updatedUser); @@ -1560,9 +1601,12 @@ public HugeUser deleteUser(Id id) { HugeUser user = this.authManager.getUser(id); E.checkArgument(!HugeAuthenticator.USER_ADMIN.equals(user.name()), "Can't delete user '%s'", user.name()); - E.checkArgument(HugeAuthenticator.USER_ADMIN.equals(currentUsername()), + String username = currentUsername(); + E.checkArgument(HugeAuthenticator.USER_ADMIN.equals(username) || + this.authManager.isAdminManager(username), "only admin can delete user", user.name()); - HugeGraphAuthProxy.this.auditLimiters.invalidate(user.id()); + HugeGraphAuthProxy.this.auditLimiters.invalidate( + auditLimiterKey(user.name())); this.invalidRoleCache(); return this.authManager.deleteUser(id); } @@ -2006,9 +2050,12 @@ public UserWithRole validateUser(String username, String password) { try { Id userKey = IdGenerator.of(username + password); - return HugeGraphAuthProxy.this.usersRoleCache.getOrFetch(userKey, id -> { - return this.authManager.validateUser(username, password); - }); + UserWithRole user = + HugeGraphAuthProxy.this.usersRoleCache.getOrFetch( + userKey, id -> this.authManager.validateUser( + username, password)); + HugeGraphAuthProxy.this.prepareAuditLimiter(user); + return user; } catch (Exception e) { LOG.error("Failed to validate user {} with error: ", username, e); @@ -2025,9 +2072,12 @@ public UserWithRole validateUser(String token) { try { Id userKey = IdGenerator.of(token); - return HugeGraphAuthProxy.this.usersRoleCache.getOrFetch(userKey, id -> { - return this.authManager.validateUser(token); - }); + UserWithRole user = + HugeGraphAuthProxy.this.usersRoleCache.getOrFetch( + userKey, + id -> this.authManager.validateUser(token)); + HugeGraphAuthProxy.this.prepareAuditLimiter(user); + return user; } catch (Exception e) { LOG.error("Failed to validate token with error: ", e); throw e; @@ -2327,7 +2377,9 @@ public TraversalStrategiesProxy(TraversalStrategies strategies) { @Override public List> toList() { - return this.strategies.toList(); + List> proxies = new ArrayList<>(); + this.iterator().forEachRemaining(proxies::add); + return proxies; } @Override @@ -2414,6 +2466,11 @@ public void apply(Traversal.Admin traversal) { */ String caller = Thread.currentThread().getName(); if (!caller.contains(TraversalStrategiesProxy.REST_WORKER)) { + for (HugePermission permission : + traversalPermissions(traversal)) { + verifyNamePermission(permission, ResourceType.GREMLIN, + script); + } verifyNamePermission(HugePermission.EXECUTE, ResourceType.GREMLIN, script); } @@ -2461,4 +2518,36 @@ public String toString() { return this.origin.toString(); } } + + private static Set traversalPermissions( + Traversal.Admin traversal) { + Set permissions = EnumSet.noneOf(HugePermission.class); + collectTraversalPermissions(traversal, permissions); + return permissions; + } + + private static void collectTraversalPermissions( + Traversal.Admin traversal, + Set permissions) { + for (Step step : traversal.getSteps()) { + if (step instanceof AddVertexStartStep || + step instanceof AddVertexStep || + step instanceof AddEdgeStartStep || + step instanceof AddEdgeStep || + step instanceof AddPropertyStep) { + permissions.add(HugePermission.WRITE); + } else if (step instanceof DropStep) { + permissions.add(HugePermission.DELETE); + } + if (step instanceof TraversalParent) { + TraversalParent parent = (TraversalParent) step; + for (Traversal.Admin child : parent.getLocalChildren()) { + collectTraversalPermissions(child, permissions); + } + for (Traversal.Admin child : parent.getGlobalChildren()) { + collectTraversalPermissions(child, permissions); + } + } + } + } } diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java index 96717e7240..848eeee8cc 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java @@ -1314,9 +1314,6 @@ public HugeGraph createGraph(String graphSpace, String name, String creator, throw new ExistedException("graph", key); } boolean grpcThread = Thread.currentThread().getName().contains("grpc"); - if (grpcThread) { - HugeGraphAuthProxy.setAdmin(); - } E.checkArgumentNotNull(name, "The graph name can't be null"); checkGraphName(name); String nickname; @@ -1426,9 +1423,6 @@ public HugeGraph createGraph(String graphSpace, String name, String creator, String schemas = this.schemaTemplate(graphSpace, schema).schema(); prepareSchema(graph, schemas); } - if (grpcThread) { - HugeGraphAuthProxy.resetContext(); - } return graph; } @@ -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")) { + HugeGraphAuthProxy.runAsAdmin(() -> this.consumer.accept(t)); + } else { + this.consumer.accept(t); } - consumer.accept(t); } catch (Throwable e) { LOG.error("Listener exception occurred.", e); - } finally { - if (grpcThread) { - HugeGraphAuthProxy.resetContext(); - } } } } @@ -2498,11 +2487,6 @@ private void graphAddHandler(T response) { // TODO: add alias graph graph = this.createGraph(parts[0], parts[1], creator, config, false); LOG.info("Add graph space:{} graph:{}", parts[0], parts[1]); - // TODO: use a more secure method to determine administrator privileges - boolean grpcThread = Thread.currentThread().getName().contains("grpc"); - if (grpcThread) { - HugeGraphAuthProxy.setAdmin(); - } graph.started(true); if (graph.tx().isOpen()) { graph.tx().close(); diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/version/ApiVersion.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/version/ApiVersion.java index 7e314f9ed6..00e8dad032 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/version/ApiVersion.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/version/ApiVersion.java @@ -121,6 +121,7 @@ public final class ApiVersion { * [0.69] Issue-1748: Support Cypher query RESTful API * [0.70] PR-2242: Add edge-existence RESTful API * [0.71] PR-2286: Support Arthas API & Metric API prometheus format + * [0.72] Support GraphSpace-wide default-role management APIs */ /** diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/StandardAuthManagerV2.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/StandardAuthManagerV2.java index 1f34aa4593..aaf2a9df17 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/StandardAuthManagerV2.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/StandardAuthManagerV2.java @@ -1815,7 +1815,10 @@ public Id createSpaceDefaultRole(String graphSpace, String owner, @Override public boolean isDefaultRole(String graphSpace, String owner, HugeDefaultRole role) { - return isDefaultRole(graphSpace, owner, role.toString()); + String roleName = role.isGraphRole() ? + getGraphDefaultRole(ALL_GRAPHS, role.toString()) : + role.toString(); + return isDefaultRole(graphSpace, owner, roleName); } @Override @@ -1828,7 +1831,10 @@ public boolean isDefaultRole(String graphSpace, String graph, @Override public void deleteDefaultRole(String graphSpace, String owner, HugeDefaultRole role) { - deleteDefaultRoleByName(graphSpace, owner, role.toString()); + String roleName = role.isGraphRole() ? + getGraphDefaultRole(ALL_GRAPHS, role.toString()) : + role.toString(); + deleteDefaultRoleByName(graphSpace, owner, roleName); } @Override diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java index 1733680e3f..da55301deb 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java @@ -31,6 +31,7 @@ import org.apache.hugegraph.unit.api.filter.PathFilterTest; import org.apache.hugegraph.unit.api.gremlin.GremlinQueryAPITest; import org.apache.hugegraph.unit.api.space.GraphSpaceAPITest; +import org.apache.hugegraph.unit.api.space.SchemaTemplateAPITest; import org.apache.hugegraph.unit.auth.HugeGraphAuthProxyTest; import org.apache.hugegraph.unit.cache.CacheManagerTest; import org.apache.hugegraph.unit.cache.CacheTest; @@ -110,6 +111,7 @@ /* api space */ GraphSpaceAPITest.class, + SchemaTemplateAPITest.class, /* cache */ CacheTest.RamCacheTest.class, diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/GraphSpaceAPITest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/GraphSpaceAPITest.java index caa659a4d4..6315f3ae7e 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/GraphSpaceAPITest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/GraphSpaceAPITest.java @@ -87,6 +87,81 @@ public void testAdminCanCheckSpaceDefaultRole() { Assert.assertContains("\"check\":true", result); } + @Test + public void testAdminCanCheckSpaceWideObserverRole() { + GraphSpaceAPI api = new GraphSpaceAPI(); + GraphManager manager = managerWithDefaultRoleContext(ADMIN, true); + setContext(ADMIN); + + String result = api.checkDefaultRole(manager, GRAPHSPACE, TARGET, + "OBSERVER", null); + + Assert.assertContains("\"check\":true", result); + } + + @Test + public void testCurrentUserCanCheckSpaceWideObserverRole() { + ManagerAPI api = new ManagerAPI(); + GraphManager manager = managerWithDefaultRoleContext(TARGET, false); + setContext(TARGET); + + String result = api.checkDefaultRole(manager, GRAPHSPACE, + "OBSERVER", null); + + Assert.assertContains("\"check\":true", result); + } + + @Test + public void testCurrentUserObserverCheckFallsBackToLegacyGraphRole() { + ManagerAPI api = new ManagerAPI(); + GraphManager manager = managerWithDefaultRoleContext(TARGET, false); + AuthManager auth = manager.authManager(); + Mockito.when(auth.isDefaultRole( + GRAPHSPACE, TARGET, HugeDefaultRole.OBSERVER)) + .thenReturn(false); + setContext(TARGET); + + String result = api.checkDefaultRole(manager, GRAPHSPACE, + "OBSERVER", null); + + Assert.assertContains("\"check\":true", result); + Mockito.verify(auth).isDefaultRole( + GRAPHSPACE, GRAPH, TARGET, HugeDefaultRole.OBSERVER); + } + + @Test + public void testObserverCheckFallsBackToLegacyGraphRole() { + GraphSpaceAPI api = new GraphSpaceAPI(); + GraphManager manager = managerWithDefaultRoleContext(ADMIN, true); + AuthManager auth = manager.authManager(); + Mockito.when(auth.isDefaultRole( + GRAPHSPACE, TARGET, HugeDefaultRole.OBSERVER)) + .thenReturn(false); + setContext(ADMIN); + + String result = api.checkDefaultRole(manager, GRAPHSPACE, TARGET, + "OBSERVER", null); + + Assert.assertContains("\"check\":true", result); + Mockito.verify(auth).isDefaultRole( + GRAPHSPACE, GRAPH, TARGET, HugeDefaultRole.OBSERVER); + } + + @Test + public void testObserverDeleteCleansSpaceAndLegacyGraphRoles() { + GraphSpaceAPI api = new GraphSpaceAPI(); + GraphManager manager = managerWithDefaultRoleContext(ADMIN, true); + AuthManager auth = manager.authManager(); + setContext(ADMIN); + + api.deleteDefaultRole(manager, GRAPHSPACE, TARGET, "OBSERVER", null); + + Mockito.verify(auth).deleteDefaultRole( + GRAPHSPACE, TARGET, HugeDefaultRole.OBSERVER); + Mockito.verify(auth).deleteDefaultRole( + GRAPHSPACE, TARGET, HugeDefaultRole.OBSERVER, GRAPH); + } + @Test public void testManagerDefaultRoleRejectsMissingGraphSpace() { ManagerAPI api = new ManagerAPI(); @@ -191,6 +266,9 @@ private static GraphManager managerWithDefaultRoleContext(String operator, Mockito.when(authManager.isDefaultRole(GRAPHSPACE, TARGET, HugeDefaultRole.SPACE)) .thenReturn(true); + Mockito.when(authManager.isDefaultRole(GRAPHSPACE, TARGET, + HugeDefaultRole.OBSERVER)) + .thenReturn(true); Mockito.when(authManager.findUser(TARGET)) .thenReturn(new HugeUser(TARGET)); @@ -206,7 +284,8 @@ private static GraphManager managerWithDefaultRoleContext(String operator, MetaManager metaManager = Mockito.mock(MetaManager.class); Mockito.when(metaManager.graphConfigs(GRAPHSPACE)) - .thenReturn(Collections.emptyMap()); + .thenReturn(Collections.singletonMap( + GRAPHSPACE + "-" + GRAPH, Collections.emptyMap())); Whitebox.setInternalState(manager, "metaManager", metaManager); Map graphs = new ConcurrentHashMap<>(); diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/SchemaTemplateAPITest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/SchemaTemplateAPITest.java new file mode 100644 index 0000000000..617957fa5f --- /dev/null +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/SchemaTemplateAPITest.java @@ -0,0 +1,81 @@ +/* + * 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.hugegraph.unit.api.space; + +import java.util.function.Supplier; + +import org.apache.hugegraph.api.space.SchemaTemplateAPI; +import org.apache.hugegraph.auth.AuthManager; +import org.apache.hugegraph.testutil.Assert; +import org.apache.hugegraph.testutil.Whitebox; +import org.junit.Test; +import org.mockito.Mockito; + +public class SchemaTemplateAPITest { + + private static final String GRAPHSPACE = "space"; + private static final String CREATOR = "creator"; + + @Test + public void testCreatorCanManageTemplate() { + Supplier authManager = + Mockito.mock(Supplier.class); + + Assert.assertTrue(canManage(authManager, CREATOR)); + Mockito.verifyZeroInteractions(authManager); + } + + @Test + public void testGlobalAdminCanManageAnotherUsersTemplate() { + Assert.assertTrue(canManage(authManager(true, false), "admin")); + } + + @Test + public void testSpaceManagerCanManageAnotherUsersTemplate() { + Assert.assertTrue(canManage(authManager(false, true), + "space-admin")); + } + + @Test + public void testUnrelatedUserCannotManageTemplate() { + Assert.assertFalse(canManage(authManager(false, false), "member")); + } + + private static Supplier authManager(boolean admin, + boolean spaceManager) { + AuthManager auth = Mockito.mock(AuthManager.class); + Mockito.when(auth.isAdminManager(Mockito.anyString())) + .thenReturn(admin); + Mockito.when(auth.isSpaceManager(GRAPHSPACE, "space-admin")) + .thenReturn(spaceManager); + return () -> auth; + } + + private static boolean canManage( + Supplier authManager, + String username) { + return Whitebox.invokeStatic( + SchemaTemplateAPI.class, + new Class[]{Supplier.class, String.class, + String.class, String.class}, + "canManage", + authManager, GRAPHSPACE, CREATOR, username); + } +} diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/HugeGraphAuthProxyTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/HugeGraphAuthProxyTest.java index 1b209c9139..399c685fb2 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/HugeGraphAuthProxyTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/HugeGraphAuthProxyTest.java @@ -19,22 +19,32 @@ import java.lang.reflect.Method; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; import org.apache.hugegraph.HugeGraph; import org.apache.hugegraph.auth.AuthManager; import org.apache.hugegraph.auth.HugeAuthenticator; import org.apache.hugegraph.auth.HugeDefaultRole; import org.apache.hugegraph.auth.HugeGraphAuthProxy; +import org.apache.hugegraph.auth.HugePermission; +import org.apache.hugegraph.auth.HugeUser; +import org.apache.hugegraph.auth.ResourceObject; import org.apache.hugegraph.auth.RolePermission; import org.apache.hugegraph.auth.UserWithRole; +import org.apache.hugegraph.backend.cache.Cache; +import org.apache.hugegraph.backend.id.Id; import org.apache.hugegraph.backend.id.IdGenerator; import org.apache.hugegraph.config.AuthOptions; import org.apache.hugegraph.config.HugeConfig; import org.apache.hugegraph.task.TaskManager; import org.apache.hugegraph.task.TaskScheduler; import org.apache.hugegraph.testutil.Assert; +import org.apache.hugegraph.testutil.Whitebox; import org.apache.hugegraph.unit.BaseUnitTest; +import org.apache.hugegraph.util.RateLimiter; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.core.Filter; @@ -44,6 +54,9 @@ import org.apache.logging.log4j.core.appender.AbstractAppender; import org.apache.logging.log4j.core.config.LoggerConfig; import org.apache.logging.log4j.core.config.Property; +import org.apache.tinkerpop.gremlin.process.traversal.Traversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; import org.junit.After; import org.junit.Test; import org.mockito.Mockito; @@ -110,6 +123,76 @@ public void testUsernameWithAdminUser() { Assert.assertEquals("admin", username); } + @Test + public void testRunAsAdminRestoresContext() { + HugeAuthenticator.User user = new HugeAuthenticator.User( + "test_user", + RolePermission.admin() + ); + setContext(new HugeGraphAuthProxy.Context(user)); + + HugeGraphAuthProxy.runAsAdmin(() -> { + Assert.assertEquals(HugeAuthenticator.USER_ADMIN, + HugeGraphAuthProxy.username()); + }); + + Assert.assertEquals("test_user", HugeGraphAuthProxy.username()); + } + + @Test + public void testRunAsAdminOverridesTaskContext() { + HugeAuthenticator.User taskUser = new HugeAuthenticator.User( + "task_user", + RolePermission.admin() + ); + TaskManager.setContext(taskUser.toJson()); + + HugeGraphAuthProxy.runAsAdmin(() -> { + Assert.assertEquals(HugeAuthenticator.USER_ADMIN, + HugeGraphAuthProxy.username()); + }); + + Assert.assertEquals("task_user", HugeGraphAuthProxy.username()); + } + + @Test + public void testRunAsAdminRestoresContextAfterException() { + HugeAuthenticator.User taskUser = new HugeAuthenticator.User( + "task_user", + RolePermission.admin() + ); + TaskManager.setContext(taskUser.toJson()); + + Assert.assertThrows(RuntimeException.class, () -> { + HugeGraphAuthProxy.runAsAdmin(() -> { + throw new RuntimeException("expected"); + }); + }); + + Assert.assertEquals("task_user", HugeGraphAuthProxy.username()); + } + + @Test + public void testRunAsAdminDoesNotPropagateToChildThread() + throws InterruptedException { + AtomicReference username = new AtomicReference<>(); + + HugeGraphAuthProxy.runAsAdmin(() -> { + Thread child = new Thread(() -> { + username.set(HugeGraphAuthProxy.username()); + }); + child.start(); + try { + child.join(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException(e); + } + }); + + Assert.assertEquals("anonymous", username.get()); + } + @Test public void testGetContextReturnsNull() { // Ensure both TaskManager context and CONTEXTS are null @@ -223,6 +306,7 @@ public void testDefaultRoleMutationInvalidatesUserRoleCache() HugeConfig config = Mockito.mock(HugeConfig.class); AuthManager authManager = Mockito.mock(AuthManager.class); TaskScheduler scheduler = Mockito.mock(TaskScheduler.class); + Id storedUserId = IdGenerator.of("stored-user-id"); Mockito.when(graph.spaceGraphName()).thenReturn("hugegraph"); Mockito.when(graph.configuration()).thenReturn(config); @@ -235,7 +319,11 @@ public void testDefaultRoleMutationInvalidatesUserRoleCache() Mockito.when(config.get(AuthOptions.AUTH_AUDIT_LOG_RATE)) .thenReturn(1000D); Mockito.when(authManager.validateUser("cache_user", "pass")) - .thenReturn(new UserWithRole("cache_user")); + .thenReturn(new UserWithRole( + storedUserId, "cache_user", + RolePermission.all("hugegraph"))); + Mockito.when(authManager.validateUser("invalid", "wrong")) + .thenReturn(new UserWithRole("invalid")); Mockito.when(authManager.createDefaultRole("DEFAULT", "cache_user", HugeDefaultRole.ANALYST, "hugegraph")) @@ -244,7 +332,15 @@ public void testDefaultRoleMutationInvalidatesUserRoleCache() HugeGraphAuthProxy proxy = new HugeGraphAuthProxy(graph); AuthManager proxyAuthManager = proxy.authManager(); + proxyAuthManager.validateUser("invalid", "wrong"); proxyAuthManager.validateUser("cache_user", "pass"); + Cache auditLimiters = + Whitebox.getInternalState(proxy, "auditLimiters"); + Assert.assertFalse(auditLimiters.containsKey( + IdGenerator.of("invalid"))); + Assert.assertTrue(auditLimiters.containsKey( + IdGenerator.of("cache_user"))); + Assert.assertFalse(auditLimiters.containsKey(storedUserId)); proxyAuthManager.validateUser("cache_user", "pass"); Mockito.verify(authManager, Mockito.times(1)) .validateUser("cache_user", "pass"); @@ -263,6 +359,7 @@ public void testLogoutInvalidatesTokenRoleCache() throws Exception { AuthManager authManager = Mockito.mock(AuthManager.class); TaskScheduler scheduler = Mockito.mock(TaskScheduler.class); String token = "cached-token"; + Id storedUserId = IdGenerator.of("stored-user-id"); Mockito.when(graph.spaceGraphName()).thenReturn("hugegraph"); Mockito.when(graph.configuration()).thenReturn(config); @@ -275,11 +372,22 @@ public void testLogoutInvalidatesTokenRoleCache() throws Exception { Mockito.when(config.get(AuthOptions.AUTH_AUDIT_LOG_RATE)) .thenReturn(1000D); Mockito.when(authManager.validateUser(token)) - .thenReturn(new UserWithRole("cache_user")); + .thenReturn(new UserWithRole( + storedUserId, "cache_user", + RolePermission.all("hugegraph"))); + Mockito.when(authManager.validateUser("invalid-token")) + .thenReturn(new UserWithRole("")); - AuthManager proxyAuthManager = - new HugeGraphAuthProxy(graph).authManager(); + HugeGraphAuthProxy proxy = new HugeGraphAuthProxy(graph); + AuthManager proxyAuthManager = proxy.authManager(); + proxyAuthManager.validateUser("invalid-token"); proxyAuthManager.validateUser(token); + Cache auditLimiters = + Whitebox.getInternalState(proxy, "auditLimiters"); + Assert.assertEquals(1L, auditLimiters.size()); + Assert.assertTrue(auditLimiters.containsKey( + IdGenerator.of("cache_user"))); + Assert.assertFalse(auditLimiters.containsKey(storedUserId)); proxyAuthManager.validateUser(token); Mockito.verify(authManager, Mockito.times(1)).validateUser(token); @@ -290,6 +398,60 @@ public void testLogoutInvalidatesTokenRoleCache() throws Exception { Mockito.verify(authManager, Mockito.times(2)).validateUser(token); } + @Test + public void testDeleteUserInvalidatesUsernameAuditLimiter() { + HugeGraph graph = Mockito.mock(HugeGraph.class); + HugeConfig config = Mockito.mock(HugeConfig.class); + AuthManager authManager = Mockito.mock(AuthManager.class); + TaskScheduler scheduler = Mockito.mock(TaskScheduler.class); + Id storedUserId = IdGenerator.of("stored-user-id"); + HugeUser storedUser = new HugeUser(storedUserId, "cache_user"); + + Mockito.when(graph.spaceGraphName()).thenReturn("hugegraph"); + Mockito.when(graph.configuration()).thenReturn(config); + Mockito.when(graph.authManager()).thenReturn(authManager); + Mockito.when(graph.taskScheduler()).thenReturn(scheduler); + Mockito.when(config.get(AuthOptions.AUTH_CACHE_EXPIRE)) + .thenReturn(3600L); + Mockito.when(config.get(AuthOptions.AUTH_CACHE_CAPACITY)) + .thenReturn(100L); + Mockito.when(config.get(AuthOptions.AUTH_AUDIT_LOG_RATE)) + .thenReturn(1000D); + Mockito.when(authManager.validateUser("cache_user", "pass")) + .thenReturn(new UserWithRole( + storedUserId, "cache_user", + RolePermission.all("hugegraph"))); + Mockito.when(authManager.getUser(storedUserId)).thenReturn(storedUser); + Mockito.when(authManager.isAdminManager("custom_admin")) + .thenReturn(true); + + HugeGraphAuthProxy proxy = new HugeGraphAuthProxy(graph); + AuthManager proxyAuthManager = proxy.authManager(); + proxyAuthManager.validateUser("cache_user", "pass"); + Cache auditLimiters = + Whitebox.getInternalState(proxy, "auditLimiters"); + Assert.assertTrue(auditLimiters.containsKey( + IdGenerator.of("cache_user"))); + + setContext(new HugeGraphAuthProxy.Context( + new HugeAuthenticator.User( + HugeAuthenticator.USER_ADMIN, + RolePermission.admin()))); + proxyAuthManager.updateUser(storedUser); + + setContext(new HugeGraphAuthProxy.Context( + new HugeAuthenticator.User( + "custom_admin", + RolePermission.admin()))); + proxyAuthManager.updateUser(storedUser); + proxyAuthManager.deleteUser(storedUserId); + + Assert.assertFalse(auditLimiters.containsKey( + IdGenerator.of("cache_user"))); + Mockito.verify(authManager, Mockito.times(2)).updateUser(storedUser); + Mockito.verify(authManager).deleteUser(storedUserId); + } + @Test public void testProxyOverridesEveryScopedDefaultMethod() throws Exception { HugeGraph graph = Mockito.mock(HugeGraph.class); @@ -366,6 +528,139 @@ public void testValidateUserDoesNotLogBearerToken() { } } + @Test + public void testTraversalPermissions() throws Exception { + Traversal.Admin read = __.V().asAdmin(); + Assert.assertTrue(traversalPermissions(read).isEmpty()); + + Traversal.Admin write = + __.addV("person").property("name", "marko").asAdmin(); + Assert.assertEquals(Collections.singleton(HugePermission.WRITE), + traversalPermissions(write)); + + Traversal.Admin delete = __.V().drop().asAdmin(); + Assert.assertEquals(Collections.singleton(HugePermission.DELETE), + traversalPermissions(delete)); + + Traversal.Admin parent = + __.V().sideEffect(__.addE("knows")).asAdmin(); + Assert.assertEquals(Collections.singleton(HugePermission.WRITE), + traversalPermissions(parent)); + } + + @Test + public void testTraversalStrategyListKeepsAuthProxy() { + HugeGraph graph = Mockito.mock(HugeGraph.class); + HugeConfig config = Mockito.mock(HugeConfig.class); + AuthManager authManager = Mockito.mock(AuthManager.class); + TaskScheduler scheduler = Mockito.mock(TaskScheduler.class); + + Mockito.when(graph.spaceGraphName()).thenReturn("hugegraph"); + Mockito.when(graph.configuration()).thenReturn(config); + Mockito.when(graph.authManager()).thenReturn(authManager); + Mockito.when(graph.taskScheduler()).thenReturn(scheduler); + Mockito.when(config.get(AuthOptions.AUTH_CACHE_EXPIRE)) + .thenReturn(3600L); + Mockito.when(config.get(AuthOptions.AUTH_CACHE_CAPACITY)) + .thenReturn(100L); + Mockito.when(config.get(AuthOptions.AUTH_AUDIT_LOG_RATE)) + .thenReturn(1000D); + + GraphTraversalSource traversal = + new HugeGraphAuthProxy(graph).traversal(); + Assert.assertFalse(traversal.getStrategies().toList().isEmpty()); + traversal.getStrategies().toList().forEach(strategy -> { + Assert.assertEquals("TraversalStrategyProxy", + strategy.getClass().getSimpleName()); + }); + } + + @Test + public void testSpaceMemberDoesNotGrantMutationPermissions() { + RolePermission role = RolePermission.fromJson( + "{\"roles\":{\"DEFAULT\":{\"*\":{" + + "\"READ\":{\"ALL\":[{\"type\":\"ALL\"}]}," + + "\"SPACE_MEMBER\":{\"ALL\":[{\"type\":\"ALL\"}]}" + + "}}}}"); + HugeAuthenticator.RequiredPerm read = + new HugeAuthenticator.RequiredPerm() + .graphSpace("DEFAULT") + .owner("hugegraph") + .action("read"); + HugeAuthenticator.RequiredPerm write = + new HugeAuthenticator.RequiredPerm() + .graphSpace("DEFAULT") + .owner("hugegraph") + .action("write"); + HugeAuthenticator.RequiredPerm delete = + new HugeAuthenticator.RequiredPerm() + .graphSpace("DEFAULT") + .owner("hugegraph") + .action("delete"); + + Assert.assertTrue(HugeAuthenticator.RolePerm.matchApiRequiredPerm( + role, read)); + Assert.assertFalse(HugeAuthenticator.RolePerm.matchApiRequiredPerm( + role, write)); + Assert.assertFalse(HugeAuthenticator.RolePerm.matchApiRequiredPerm( + role, delete)); + } + + @Test + public void testSpaceManagerCanManageUserGrantInOwnSpace() { + RolePermission managerRole = RolePermission.fromJson( + "{\"roles\":{\"space-a\":{\"*\":{" + + "\"SPACE\":{\"ALL\":[{\"type\":\"ALL\"}]}" + + "}}}}"); + RolePermission memberGrant = RolePermission.fromJson( + "{\"roles\":{\"space-a\":{\"*\":{" + + "\"READ\":{\"ALL\":[{\"type\":\"ALL\"}]}," + + "\"WRITE\":{\"ALL\":[{\"type\":\"ALL\"}]}" + + "}}}}"); + RolePermission otherSpaceGrant = RolePermission.fromJson( + "{\"roles\":{\"space-b\":{\"*\":{" + + "\"READ\":{\"ALL\":[{\"type\":\"ALL\"}]}," + + "\"WRITE\":{\"ALL\":[{\"type\":\"ALL\"}]}" + + "}}}}"); + RolePermission multiSpaceGrant = RolePermission.fromJson( + "{\"roles\":{" + + "\"space-a\":{\"*\":{" + + "\"READ\":{\"ALL\":[{\"type\":\"ALL\"}]}}}," + + "\"space-b\":{\"*\":{" + + "\"READ\":{\"ALL\":[{\"type\":\"ALL\"}]}}}" + + "}}"); + HugeUser member = new HugeUser("member"); + ResourceObject ownSpace = + ResourceObject.of("space-a", "hugegraph", member); + ResourceObject otherSpace = + ResourceObject.of("space-b", "hugegraph", member); + ResourceObject admin = + ResourceObject.of("space-a", "hugegraph", + new HugeUser(HugeAuthenticator.USER_ADMIN)); + + Assert.assertTrue(HugeAuthenticator.RolePerm.match( + managerRole, memberGrant, ownSpace)); + Assert.assertFalse(HugeAuthenticator.RolePerm.match( + managerRole, memberGrant, otherSpace)); + Assert.assertFalse(HugeAuthenticator.RolePerm.match( + managerRole, otherSpaceGrant, ownSpace)); + Assert.assertTrue(HugeAuthenticator.RolePerm.match( + managerRole, multiSpaceGrant, ownSpace)); + Assert.assertFalse(HugeAuthenticator.RolePerm.match( + managerRole, memberGrant, admin)); + Assert.assertFalse(HugeAuthenticator.RolePerm.match( + managerRole, RolePermission.admin(), ownSpace)); + } + + @SuppressWarnings("unchecked") + private static Set traversalPermissions( + Traversal.Admin traversal) throws Exception { + Method method = HugeGraphAuthProxy.class.getDeclaredMethod( + "traversalPermissions", Traversal.Admin.class); + method.setAccessible(true); + return (Set) method.invoke(null, traversal); + } + private static class TestAppender extends AbstractAppender { private final List events; diff --git a/style/checkstyle.xml b/style/checkstyle.xml index eec890ec25..d028e10b20 100644 --- a/style/checkstyle.xml +++ b/style/checkstyle.xml @@ -27,7 +27,7 @@ - +