VGalaxies commented on code in PR #3008:
URL: https://github.com/apache/hugegraph/pull/3008#discussion_r3371365855
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/ServerInfoManager.java:
##########
@@ -122,19 +122,25 @@ public synchronized void initServerInfo(GlobalMasterInfo
nodeInfo) {
"The server with name '%s' already in cluster",
serverId);
if (nodeInfo.nodeRole().master()) {
- String page = this.supportsPaging() ? PageInfo.PAGE_NONE : null;
- do {
- Iterator<HugeServerInfo> servers = this.serverInfos(PAGE_SIZE,
page);
- while (servers.hasNext()) {
- existed = servers.next();
- E.checkArgument(!existed.role().master() ||
!existed.alive(),
- "Already existed master '%s' in current
cluster",
- existed.id());
- }
- if (page != null) {
- page = PageInfo.pageInfo(servers);
- }
- } while (page != null);
+ try {
+ String page = this.supportsPaging() ? PageInfo.PAGE_NONE :
null;
+ do {
+ Iterator<HugeServerInfo> servers =
this.serverInfos(PAGE_SIZE, page);
+ while (servers.hasNext()) {
+ existed = servers.next();
+ E.checkArgument(!existed.role().master() ||
!existed.alive(),
+ "Already existed master '%s' in
current cluster",
+ existed.id());
+ }
+ if (page != null) {
+ page = PageInfo.pageInfo(servers);
+ }
+ } while (page != null);
+ } catch (Exception e) {
Review Comment:
High
`hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/ServerInfoManager.java:139`
- Existing live master detection is swallowed
Evidence: The uniqueness check at lines 131-133 throws when another alive
master exists, but the new broad `catch (Exception e)` at line 139 logs and
continues to `saveServerInfo()` at line 149.
Impact: A node can register itself as master even after detecting an
existing alive master, allowing multiple masters in the same cluster.
Requested fix: Do not catch the invariant failure. Only handle the
specific schema-mismatch/read error if needed, and rethrow when an existing
alive master is found.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java:
##########
@@ -103,6 +107,171 @@ public Object get(@Context GraphManager manager,
return gsInfo;
}
+ @POST
+ @Timed
+ @Status(Status.CREATED)
+ @Consumes(APPLICATION_JSON)
+ @Produces(APPLICATION_JSON_WITH_CHARSET)
+ @Path("{graphspace}/role")
+ @RolesAllowed({"analyst"})
Review Comment:
High
`hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java:116`
- Analysts can grant or revoke default roles for arbitrary users
Evidence: `POST /graphspaces/{graphspace}/role` and `DELETE
/graphspaces/{graphspace}/role` are gated only by `@RolesAllowed("analyst")`;
after that, non-admin callers are blocked only for `SPACE`, while `ANALYST`,
`SPACE_MEMBER`, and `OBSERVER` are created/deleted at lines 166-171 and
268-271. `ANALYST` grants READ/WRITE/DELETE/EXECUTE in
`StandardAuthManagerV2.java:1785-1787`.
Impact: Any caller satisfying the analyst role can grant destructive
graphspace permissions to other users or remove their roles, bypassing the
existing admin/space-manager role-management boundary.
Requested fix: Require admin or space manager permission for these role
mutations, matching `ManagerAPI`, and keep admin-only enforcement for `SPACE`
if needed.
--
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]
