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: Existing live master detection is swallowed**
`hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/ServerInfoManager.java:139`
**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: Analysts can grant or revoke default roles for arbitrary users**
`hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java:116`
**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]
