Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3273 +/- ##
============================================
- Coverage 58.42% 58.39% -0.03%
+ Complexity 6903 3079 -3824
============================================
Files 779 779
Lines 69391 69414 +23
Branches 9567 9572 +5
============================================
- Hits 40539 40537 -2
- Misses 25179 25200 +21
- Partials 3673 3677 +4
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: The Ranger authorizer hook can be bypassed. When its rate limiter has no permit left it allows the request, so per-label deny policies stop working for list reads. Separately, init-store now creates the admin with the public default password pa for every built-in-auth install, whether or not Ranger is used. Within the plugin, the default-graph and default-role calls fail through reflection, Ranger grants never apply outside graph space DEFAULT, the pinned Ranger 2.4.0 has a critical CVE, and CI never builds the module. The branch also carries 37 commits already on master, replayed with new SHAs (#3140 to #3218), so GitHub shows 370 files. The PR conflicts with master (mergeStateStatus DIRTY). Rebasing onto current master would cut the diff down to the 31 files this PR really changes. Evidence: compared head 4057f3f with upstream 0a3e4ae (#3218, the last replayed commit). That diff is the full delta: 31 files, +8697/-9. I read every hand-written Java, POM, Dockerfile and compose file in it, plus the service definition and the integration README's policy model. I traced the callers in HugeGraphAuthProxy, HugeAuthenticator.RolePerm, GraphManager, AuthenticationFilter, GraphsAPI, GraphSpaceAPI and InitStore. Local repros: Guava RateLimiter.create(1000.0) granted 2 of 10000 back-to-back tryAcquire() calls, and a JDK 17 two-package repro showed that a reflective call on a package-private class throws IllegalAccessException. CI: dependency-review fails on GHSA-c87w-642h-m97h, which this PR introduces. dependency-check failed on a Maven Central 429, which is not caused by this PR.
| return RateLimiter.create(this.auditLogMaxRate); | ||
| }); | ||
| if (!limiter.tryAcquire()) { | ||
| return true; |
There was a problem hiding this comment.
Critical: Blocking: yes. Summary: When the per-shape rate limiter has no permit left, checkResourceAuthorizer() returns true without asking Ranger, so a Ranger deny is skipped for almost every check in a burst. The limiter key is username/graphSpace/graph/type, with no label or action in it. One allowed read of person uses up the permit, and a read of software in the same millisecond is then allowed even when a Ranger deny policy covers it. That is the setup the integration guide uses to narrow access (a wildcard allow plus a per-label deny). Listing vertices or edges checks every element back to back, so a plain GET .../graph/vertices returns the denied labels. Evidence: auth.audit_log_rate defaults to 1000/s, so RateLimiter.create(1000) hands out one permit per millisecond. A local run with Guava 32.1.2 that called tryAcquire() 10000 times in a loop on RateLimiter.create(1000.0) got a permit 2 times and false 9998 times. This method turns every one of those false results into an allow. Fix: always ask the authorizer for the decision. If audit volume is the concern, rate-limit the audit output, or cache the decision per user, graph space, graph, type, label and action. Never fall back to allow.
| @@ -219,8 +202,9 @@ public static void initAdminUserIfNeeded(String confFile) throws Exception { | |||
| config.addProperty(INITING_STORE, true); | |||
| auth.setup(config); | |||
| if (auth.graph().backendStoreFeatures().supportsPersistence()) { | |||
| auth.initAdminUser(); | |||
| auth.initAdminUser(config.get(ServerOptions.ADMIN_PA)); | |||
There was a problem hiding this comment.
Important: Blocking: yes. Summary: init-store.sh no longer asks for the admin password. Before this change the line called the private initAdminUser(), which reads the password from the console or stdin. It now passes auth.admin_pa, whose default is the public value pa. A bare-metal install that enables the built-in authenticator and does not set auth.admin_pa now gets an admin account with password pa, with no prompt and no warning. This affects every StandardAuthenticator user, not only Ranger users. Evidence: at 0a3e4ae5f (master's #3218, the base this PR replays) this line is auth.initAdminUser();, and that method calls this.initAdminUser(this.inputPassword()). ServerOptions.ADMIN_PA defaults to "pa", and the shipped conf/rest-server.properties leaves #auth.admin_pa=pa commented out. hugegraph-dist/docker/docker-entrypoint.sh (line 222) still documents that init-store reads the password from stdin. With this change, initAdminUser() and inputPassword() are dead code. Fix: use auth.admin_pa only when the file sets it explicitly (config.containsKey(ServerOptions.ADMIN_PA.name()) with a non-empty value, the same test InitStore.checkAdminBootstrapReachable uses), and otherwise keep the prompt. The Ranger stack sets auth.admin_pa=admin, so it would still work.
| Class<?>[] paramTypes, Object... args) { | ||
| try { | ||
| java.lang.reflect.Method m = | ||
| delegate.getClass().getMethod(methodName, paramTypes); |
There was a problem hiding this comment.
Important: Blocking: yes. Summary: With the Ranger authenticator enabled, every default-graph and default-role call fails with IllegalAccessException, so those REST endpoints return 500. The delegate is HugeGraphAuthProxy.AuthManagerProxy, a package-private inner class in org.apache.hugegraph.auth. getClass().getMethod(...) returns the method declared on that class, and Method.invoke from org.apache.hugegraph.ranger is refused. The affected callers reach this class through GraphManager.authManager(): GraphsAPI (setDefaultGraph, getDefaultGraph), GraphSpaceAPI and ManagerAPI (createDefaultRole, createSpaceDefaultRole, isDefaultRole, deleteDefaultRole). Evidence: RangerHugeGraphAuthenticator.setup() passes this.graph.authManager() as the delegate. With gremlin.graph=org.apache.hugegraph.auth.HugeFactoryAuthProxy, that is AuthManagerProxy, declared as class AuthManagerProxy implements AuthManager (HugeGraphAuthProxy.java line 1592). A two-package repro of the same shape on JDK 17 throws IllegalAccessException: class b.Main cannot access a member of class a.Outer$AuthManagerProxy with modifiers "public", while the direct interface call works. All of these methods are already declared on AuthManager at this head (AuthManager.java lines 259-281), so the reflection is not needed. Fix: call delegate.setDefaultGraph(...) and the others directly, mark them @Override, and remove callDelegate.
| // round-trip (the only public merge path available outside the pkg). | ||
| // Use "DEFAULT" as graphspace key — RolePerm.matchResource() does an | ||
| // exact key lookup for requiredResource.graphSpace(), which is "DEFAULT". | ||
| StringBuilder sb = new StringBuilder("{\"roles\":{\"DEFAULT\":{\"*\":{"); |
There was a problem hiding this comment.
Important: Blocking: yes. Summary: The role built from the Ranger grants is keyed only under graph space DEFAULT. RolePerm.matchResource() looks up roles.get(requiredResource.graphSpace()) by exact key, so a user whose access comes only from Ranger is refused in every other graph space, even under a graphspace=* policy. The per-request authorizer never gets a say, because verifyResPermission consults it only after the local match has already passed. In a PD/HStore deployment with several graph spaces, Ranger cannot grant anything outside DEFAULT. Evidence: this line hard-codes {"roles":{"DEFAULT":{"*":{...}}}}. HugeAuthenticator.java line 438 is this.roles.get(requiredResource.graphSpace()), which returns false when the result is null. In HugeGraphAuthProxy.verifyResPermission the authorizer branch is guarded by if (matched && RESOURCE_AUTHORIZER != null). Fix: build the role for the graph space being accessed (or for every graph space the user can reach), and add a test where a Ranger-only user reads a graph in a graph space other than DEFAULT.
| </property> | ||
| </activation> | ||
| <modules> | ||
| <module>hugegraph-ranger-plugin</module> |
There was a problem hiding this comment.
Important: Blocking: yes. Summary: The new module is built only with -Dwith-ranger-plugin, and no workflow passes that flag. CI never compiles the module, and RangerHugeGraphPluginTest never runs. The comment above names the trade-off, but it means the next change to AuthManager or HugeGraphAuthProxy can break RangerAuthManager without anyone noticing. The authorizer branch added to HugeGraphAuthProxy.verifyResPermission also has no test: no test registers a ResourceAuthorizer. A test that did would have caught the rate-limiter allow above. Evidence: git grep ranger -- .github/ at this head returns nothing, and every check run on this head builds only the default reactor. Fix: add a CI job (or a server-ci step) that builds and tests the module with -Dwith-ranger-plugin. Also add a HugeGraphAuthProxyTest case that registers an authorizer denying one label and checks that a burst of element reads filters out every element with that label.
|
|
||
| // 401/403 mean HugeGraph is up but rejected the configured | ||
| // credentials — the endpoint is still reachable. | ||
| if (code == 200 || code == 401 || code == 403) { |
There was a problem hiding this comment.
Minor: Test Connection reports success on HTTP 401 and 403. A wrong username or password in the service config therefore looks fine in Ranger Admin, and then policy-editor autocomplete quietly returns empty lists, because getJson() throws on any non-200 and lookupResource() swallows the error. Fix: report 401 and 403 as a failed check, with a message saying the credentials were rejected.
| UserWithRole base = (token != null && !token.isEmpty()) ? | ||
| delegate.validateUser(token) : | ||
| delegate.validateUser(username, password); | ||
| return enrichWithRanger(base); |
There was a problem hiding this comment.
Minor: Every authenticated REST request goes through this method, and enrichWithRanger() then makes up to five isAccessAllowed calls (ADMIN, READ, WRITE, DELETE and EXECUTE on */*/all/*). RangerHugeGraphPlugin.init() sets RangerDefaultAuditHandler, so each of these calls is audited. Ranger therefore gets up to five synthetic audit records per request, on top of the real per-resource checks. Evidence: AuthenticationFilter.filter() calls authenticate() on every non-whitelisted request, which reaches GraphManager.authenticate(), then RangerHugeGraphAuthenticator.authenticate(), then this method. The probes are in mergeRangerRole() (lines 202-228). Fix: cache the merged role per user for a short TTL, the way usersRoleCache does for the local role, or turn off auditing for these probe requests.
apache#3272 - Ranger Plugin implementation
apache#3272 - Fixed CI Errors. THe ranger plugin is opt-in, ensured that flag is placed at all places so that unrequired dependencies are not shipped in in normal flows.
- Fix minor issues
…- Rebase onto master, Ranger 2.9.0 on Java 17 Adapt to master after rebase: - ResourceObject: Nameable -> Namifiable; RangerAuthManager: Id moved to org.apache.hugegraph.id. - Ranger plugin: drop the maven-shade-plugin 3.4.1 pin and inherit 3.6.1 (3.4.1 cannot read the Java 24 classes in byte-buddy 1.17.6). Move to Ranger 2.9.0 running on Java 17: - ranger.version 2.4.0 -> 2.9.0; add ranger-audit-dest-solr (audit destinations are separate artifacts since 2.7) and exclude its slf4j-api 2.x, which turned all HugeGraph server logging into NOP. - RangerHugeGraphService: decrypt the service password Ranger Admin passes re-encrypted (resource lookup always got HTTP 401), and make Test Connection probe an authenticated endpoint and fail on 401/403. Add RangerHugeGraphServiceTest. Integration stack (docker/hgRangerIntegration): - Build and run on Java 17; Ranger image on apache/ranger:2.9.0 with a Temurin 17 JRE; copy the plugin jar without a hardcoded version. - docker-compose: pull_policy build, Ranger 2.9.0 credential variables, DB-backed healthcheck. - Test stack only: allow unauthenticated policy download, which Ranger 2.9 rejects by default. - README: create the service instance before verifying lookups, register bob/alice in Ranger before creating policies, versions and troubleshooting updates. Remove the unrelated AML showcase notebook (docker/aml_sql_showcase_hugegraph.ipynb) and its RAT excludes; license-eye cannot check .ipynb headers.
…- Never skip Ranger decisions under audit rate limiting HugeGraphAuthProxy.checkResourceAuthorizer() returned "allowed" without asking the external authorizer once the per-shape rate limiter was exhausted. The limiter key ignored label and action, so after enough allowed checks a user could pass a per-label deny, or write/delete without a Ranger decision, for the rest of the window. - ResourceAuthorizer.isAllowed() takes an audit flag; the decision must not depend on it. - Non-admin checks always ask the authorizer; the limiter only decides whether the check is audited. Admin checks stay audit-only and are skipped once the limiter is exhausted. - RangerHugeGraphPlugin evaluates unaudited checks with a null result processor, which skips the audit handler but not the policy. - Tests: authorizer can deny a locally allowed request, every call is decided when auditing is rate-limited, admin is never denied, and ResourceObject.label() for each resource kind.
Purpose of the PR
Main Changes
Added a new module:
hugegraph-server/hugegraph-ranger-pluginRangerHugeGraphAuthenticator: plugs Ranger into HugeGraph auth flow viaauth.authenticator.RangerAuthManager: delegates identity CRUD/login to existing HugeGraph auth manager, then merges Ranger grants intoRolePermission.RangerHugeGraphPlugin: wrapsRangerBasePlugin, maps HugeGraph resource/permission model to Ranger access requests, and enables audit result processing.RangerHugeGraphService: Ranger Admin-side service for Test Connection and policy editor resource autocomplete (graphspace -> graph -> resource-type -> label).RangerOptions: adds keys forauth.ranger.service_nameandauth.ranger.config.ranger-servicedef-hugegraph.jsonranger-hugegraph-security.xmlRangerHugeGraphPluginTest.Added end-to-end integration assets under
docker/hgRangerIntegrationDockerfile.hugegraph: builds HugeGraph image with Ranger plugin enabled.Dockerfile.ranger: bakes thin HugeGraph Ranger service jar into Ranger Admin classpath.docker-compose.yml: 4-service stack (Ranger Admin + Postgres + Solr + HugeGraph).ranger-audits-conf/*,solr-init.sh,ranger-hugegraph-security.xml,rest-server.properties,hugegraph.properties).docker/hgRangerIntegration/README.mddocker/hgRangerIntegration/ranger-integration-test-guide.mdPackaging/compatibility notes
Verifying these changes
mvn -pl hugegraph-server/hugegraph-ranger-plugin -am -Dtest=RangerHugeGraphPluginTest -DfailIfNoTests=false test -DskipITs -DskipIntegrationTests -Drat.skip=true -Dcheckstyle.skip=trueTests run: 4, Failures: 0, Errors: 0, Skipped: 0)docker/hgRangerIntegration/README.mdordocker/hgRangerIntegration/ranger-integration-test-guide.mdfor policy enforcement, cache-refresh, dynamic policy update, and audit checks.DDoes this PR potentially affect the following parts?
Documentation Status
Doc - TODO: required documentation is pending; complete it before merging.Doc - Done: documentation is included here or linked below.Doc - No Need: no user-visible documentation is affected.Documentation files in this PR or paired hugegraph-doc PR:
docker/hgRangerIntegration/README.mdhugegraph-server/hugegraph-ranger-plugin/src/main/resources/ranger-servicedef-hugegraph.jsonhugegraph-server/hugegraph-ranger-plugin/src/main/resources/ranger-hugegraph-security.xmlVisual overview
Proposed integration architecture, not a claim of verified policy enforcement or audit completeness.
Conceptual illustrations; not test, benchmark, security, or release-acceptance evidence.