This is an automated email from the ASF dual-hosted git repository. imbajin pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/hugegraph-toolchain.git
commit de0fd460c07b35d1ae16f6671580164bb0a84372 Author: dark <[email protected]> AuthorDate: Tue Aug 18 22:49:44 2026 +0800 fix(hubble): secure scoped GraphSpace workflows - hide backend bootstrap details - enforce direct and indirect GraphSpace scope - isolate query records and account security updates - retain non-auth verification evidence --- .../images/pr27/15-standalone-nonauth-visual.png | Bin 0 -> 90401 bytes .../images/pr27/18-standalone-nonauth-visual.png | Bin 0 -> 93329 bytes .../java/org/apache/hugegraph/common/Constant.java | 2 + .../hugegraph/controller/BaseController.java | 38 ++++- .../hugegraph/controller/ConfigController.java | 6 +- .../controller/graphs/GraphsController.java | 10 +- .../controller/ingest/IngestController.java | 100 +++++++++++- .../controller/load/FileMappingController.java | 11 ++ .../controller/load/FileUploadController.java | 4 + .../controller/load/JobManagerController.java | 6 +- .../controller/load/LoadTaskController.java | 7 + .../controller/query/ExecuteHistoryController.java | 9 +- .../query/GremlinCollectionController.java | 30 +++- .../controller/space/GraphSpaceController.java | 35 +++-- .../controller/space/VermeerController.java | 2 +- .../hugegraph/handler/CustomInterceptor.java | 44 +++++- .../apache/hugegraph/handler/LoginInterceptor.java | 3 +- .../service/auth/GraphSpaceUserService.java | 3 +- .../apache/hugegraph/service/auth/UserService.java | 87 +++++----- .../hugegraph/service/load/JobManagerService.java | 34 ++++ .../service/query/ExecuteHistoryService.java | 20 ++- .../service/query/GremlinCollectionService.java | 25 ++- .../hugegraph/service/space/GraphSpaceService.java | 73 +++++++-- .../controller/ingest/IngestControllerTest.java | 118 ++++++++++++++ .../controller/space/GraphSpaceControllerTest.java | 68 ++++++++ .../service/auth/GraphSpaceUserServiceTest.java | 10 +- .../service/space/GraphSpaceServiceTest.java | 31 ++++ .../apache/hugegraph/unit/AuthSecurityTest.java | 127 +++++++++++++++ .../hugegraph/unit/ConfigControllerTest.java | 49 ++++++ .../hugegraph/unit/FileMappingDeletionTest.java | 27 +++- .../unit/GraphsControllerCanonicalTest.java | 52 +++++- .../hugegraph/unit/LoaderScopeControllerTest.java | 29 +++- .../unit/UserServiceCompatibilityTest.java | 175 +++++++++++++++++---- .../src/api/request-error-semantics.test.js | 28 ++++ hugegraph-hubble/hubble-fe/src/api/request.js | 13 +- .../src/i18n/resources/en-US/modules/pages.json | 2 + .../src/i18n/resources/zh-CN/modules/pages.json | 2 + .../hubble-fe/src/pages/Account/EditLayer.js | 17 +- .../pages/Account/account-edit-recovery.test.js | 73 +++++++++ .../hubble-fe/src/pages/Operations/NodeDetail.js | 17 +- .../src/pages/Operations/NodeDetail.test.js | 4 + 41 files changed, 1230 insertions(+), 161 deletions(-) diff --git a/hugegraph-hubble/docs/images/pr27/15-standalone-nonauth-visual.png b/hugegraph-hubble/docs/images/pr27/15-standalone-nonauth-visual.png new file mode 100644 index 000000000..0b472dac9 Binary files /dev/null and b/hugegraph-hubble/docs/images/pr27/15-standalone-nonauth-visual.png differ diff --git a/hugegraph-hubble/docs/images/pr27/18-standalone-nonauth-visual.png b/hugegraph-hubble/docs/images/pr27/18-standalone-nonauth-visual.png new file mode 100644 index 000000000..13e8bc6cf Binary files /dev/null and b/hugegraph-hubble/docs/images/pr27/18-standalone-nonauth-visual.png differ diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/common/Constant.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/common/Constant.java index 7c29e9518..63be9e458 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/common/Constant.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/common/Constant.java @@ -58,6 +58,8 @@ public final class Constant { public static final String TOKEN_KEY = "auth_token"; public static final String USERNAME_KEY = "username"; + public static final String GRAPHSPACE_ACCESS_KEY = + "validated_graphspace"; public static final int NO_LIMIT = -1; diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/BaseController.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/BaseController.java index 82446d910..1f7634785 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/BaseController.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/BaseController.java @@ -28,6 +28,7 @@ import org.apache.hugegraph.options.HubbleOptions; import org.apache.hugegraph.service.auth.UserService; import org.apache.hugegraph.service.auth.AuthModeService; import org.apache.hugegraph.service.auth.AuthContextService; +import org.apache.hugegraph.service.space.GraphSpaceService; import org.apache.commons.collections.CollectionUtils; import org.apache.hugegraph.config.HugeConfig; import org.springframework.beans.factory.annotation.Autowired; @@ -65,6 +66,8 @@ public abstract class BaseController { protected AuthModeService authMode; @Autowired protected AuthContextService authContextService; + @Autowired + protected GraphSpaceService graphSpaceAccessService; public static final String ORDER_ASC = "asc"; public static final String ORDER_DESC = "desc"; @@ -146,12 +149,14 @@ public abstract class BaseController { HttpServletRequest request = getRequest(); if (request.getAttribute("hugeClient") != null) { HugeClient client = (HugeClient) request.getAttribute("hugeClient"); + this.requireGraphSpaceAccess(client, graphSpace); client.assignGraph(graphSpace, graph); return client; } HugeClient client = this.authMode != null && this.authMode.anonymous() ? this.hugeClientPoolService.createUnauthClient(graphSpace, graph) : this.hugeClientPoolService.createAuthClient(graphSpace, graph, this.getToken()); + this.requireGraphSpaceAccess(client, graphSpace); if (graphSpace != null || graph != null) { client.assignGraph(graphSpace, graph); } @@ -160,6 +165,7 @@ public abstract class BaseController { } protected HugeClient requireAccountManager() { + this.requireAuthenticatedAuthorization(); HugeClient client = this.authClient(null, null); String level = this.userService.userLevel(client, this.getUser()); if (!"ADMIN".equals(level)) { @@ -169,6 +175,7 @@ public abstract class BaseController { } protected HugeClient requireGraphSpaceManager(String graphSpace) { + this.requireAuthenticatedAuthorization(); HugeClient client = this.authClient(null, null); if (!this.userService.isSuperAdmin(client) && !this.userService.isAssignSpaceAdmin(client, graphSpace)) { @@ -181,6 +188,7 @@ public abstract class BaseController { protected HugeClient requireGraphSpaceWrite(String graphSpace) { HugeClient client = this.authClient(null, null); + this.requireGraphSpaceAccess(client, graphSpace); this.authContextService.requireGraphSpaceWrite( client, this.getUser(), graphSpace); client.assignGraph(graphSpace, null); @@ -189,6 +197,7 @@ public abstract class BaseController { protected HugeClient requireGraphSpaceAuthorizationAdmin( String graphSpace) { + this.requireAuthenticatedAuthorization(); HugeClient client = this.authClient(null, null); if (!this.userService.isSuperAdmin(client)) { throw new ForbiddenException("Permission denied: manage authorization objects"); @@ -198,6 +207,7 @@ public abstract class BaseController { } protected HugeClient requireGraphSpaceAdministrator() { + this.requireAuthenticatedAuthorization(); HugeClient client = this.authClient(null, null); if (!this.userService.isSuperAdmin(client)) { throw new ForbiddenException( @@ -294,10 +304,36 @@ public abstract class BaseController { HugeClient client = hugeClientPoolService.create(url, graphSpace, graph, this.getToken()); - + this.requireGraphSpaceAccess(client, graphSpace); return client; } + private void requireAuthenticatedAuthorization() { + if (this.authMode != null && this.authMode.anonymous()) { + throw new ForbiddenException( + "Authentication is required for this operation"); + } + } + + protected void requireGraphSpaceAccess(HugeClient client, + String graphSpace) { + if (graphSpace == null || !config.get(HubbleOptions.PD_ENABLED)) { + return; + } + HttpServletRequest request = getRequest(); + if (graphSpace.equals( + request.getAttribute(Constant.GRAPHSPACE_ACCESS_KEY))) { + return; + } + if (this.authMode != null && this.authMode.anonymous()) { + this.graphSpaceAccessService.requirePublicSpace(client, + graphSpace); + } else { + this.graphSpaceAccessService.requireAccessibleSpace(client, + graphSpace); + } + } + public String getUrl() { boolean pdEnabled = config.get(HubbleOptions.PD_ENABLED); if (!pdEnabled) { diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/ConfigController.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/ConfigController.java index 7815e82e8..8f8f5cb6c 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/ConfigController.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/ConfigController.java @@ -39,12 +39,8 @@ public class ConfigController { @GetMapping public Map<String, Object> getConfig() { Map<String, Object> result = new HashMap<>(); - boolean pdEnabled = config.get(HubbleOptions.PD_ENABLED); - result.put("pd_enabled", pdEnabled); + result.put("pd_enabled", config.get(HubbleOptions.PD_ENABLED)); result.put("auth_enabled", config.get(HubbleOptions.AUTH_ENABLED)); - if (!pdEnabled) { - result.put("server_url", config.get(HubbleOptions.SERVER_URL)); - } return result; } } diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/graphs/GraphsController.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/graphs/GraphsController.java index 8fb8ac697..8e2bef18f 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/graphs/GraphsController.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/graphs/GraphsController.java @@ -333,9 +333,13 @@ public class GraphsController extends BaseController { public Object clone(@PathVariable("graphspace") String graphspace, @PathVariable("graph") String graph, @RequestBody GraphCloneEntity graphCloneEntity) { - return this.graphsService.clone(this.authClient(graphspace, graph), - graphCloneEntity.convertMap(graphspace, - graph)); + HugeClient client = this.authClient(graphspace, graph); + String targetGraphSpace = graphCloneEntity.getGraphSpace() == null ? + graphspace : + graphCloneEntity.getGraphSpace(); + this.requireGraphSpaceAccess(client, targetGraphSpace); + return this.graphsService.clone( + client, graphCloneEntity.convertMap(graphspace, graph)); } // //@Data diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/ingest/IngestController.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/ingest/IngestController.java index 7b7f97050..3fadd9af0 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/ingest/IngestController.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/ingest/IngestController.java @@ -283,7 +283,8 @@ public class IngestController extends BaseController { mapping.setEdgeMappings(edgeMappings); GraphConnection connection = this.graphConnection(graphSpace, graph); - HugeClient client = this.authClient(graphSpace, graph); + HugeClient client = this.requireGraphSpaceWrite(graphSpace); + client.assignGraph(graphSpace, graph); LoadTask task = this.jobManagerService.createIngestTask( job, mapping, connection, client); Map<String, Object> data = new HashMap<>(); @@ -299,9 +300,7 @@ public class IngestController extends BaseController { @RequestParam(name = "page_no", required = false, defaultValue = "1") int pageNo, @RequestParam(name = "page_size", required = false, defaultValue = "10") int pageSize) { - // list all jobs across all graphspaces - use empty strings to get all - // We need to query without graphspace/graph filter for the ingest view - IPage<JobManager> page = jobManagerService.listAll(pageNo, pageSize, query); + IPage<JobManager> page = this.visibleJobPage(pageNo, pageSize, query); IPage<TaskVO> result = page.convert(job -> { TaskVO vo = new TaskVO(); @@ -367,11 +366,18 @@ public class IngestController extends BaseController { return Response.builder().status(Constant.STATUS_NOT_FOUND) .message("Task not found: " + id).build(); } + this.requireJobAccess(job); return Response.builder().status(Constant.STATUS_OK).data(job).build(); } @DeleteMapping("/tasks/{id}") public Response deleteTask(@PathVariable("id") int id) { + JobManager job = jobManagerService.get(id); + if (job == null) { + return Response.builder().status(Constant.STATUS_NOT_FOUND) + .message("Task not found: " + id).build(); + } + this.requireJobWrite(job); jobManagerService.remove(id); return Response.builder().status(Constant.STATUS_OK).build(); } @@ -383,6 +389,7 @@ public class IngestController extends BaseController { return Response.builder().status(Constant.STATUS_NOT_FOUND) .message("Task not found: " + id).build(); } + this.requireJobWrite(job); job.setJobStatus(JobStatus.DEFAULT); jobManagerService.update(job); return Response.builder().status(Constant.STATUS_OK).build(); @@ -395,6 +402,7 @@ public class IngestController extends BaseController { return Response.builder().status(Constant.STATUS_NOT_FOUND) .message("Task not found: " + id).build(); } + this.requireJobWrite(job); job.setJobStatus(JobStatus.FAILED); jobManagerService.update(job); return Response.builder().status(Constant.STATUS_OK).build(); @@ -408,6 +416,12 @@ public class IngestController extends BaseController { @RequestParam(name = "page_no", required = false, defaultValue = "1") int pageNo, @RequestParam(name = "page_size", required = false, defaultValue = "10") int pageSize) { + JobManager job = jobManagerService.get(taskId); + if (job == null) { + return Response.builder().status(Constant.STATUS_NOT_FOUND) + .message("Task not found: " + taskId).build(); + } + this.requireJobAccess(job); List<LoadTask> tasks = loadTaskService.taskListByJob(taskId); // Manual pagination @@ -453,11 +467,18 @@ public class IngestController extends BaseController { return Response.builder().status(Constant.STATUS_NOT_FOUND) .message("Job not found: " + id).build(); } + this.requireLoadTaskAccess(task); return Response.builder().status(Constant.STATUS_OK).data(task).build(); } @DeleteMapping("/jobs/{id}") public Response deleteJob(@PathVariable("id") int id) { + LoadTask task = loadTaskService.get(id); + if (task == null) { + return Response.builder().status(Constant.STATUS_NOT_FOUND) + .message("Job not found: " + id).build(); + } + this.requireLoadTaskWrite(task); loadTaskService.remove(id); return Response.builder().status(Constant.STATUS_OK).build(); } @@ -466,7 +487,7 @@ public class IngestController extends BaseController { @GetMapping("/metrics/task") public Response metricsTask() { - List<JobManager> all = jobManagerService.listAll(); + List<JobManager> all = this.visibleJobs(""); all.forEach(jobManagerService::refreshStatus); long runningOnce = 0; @@ -506,6 +527,75 @@ public class IngestController extends BaseController { // ===== Helpers ===== + private IPage<JobManager> visibleJobPage(int pageNo, int pageSize, + String query) { + Set<String> graphSpaces = this.visibleGraphSpaces(); + IPage<JobManager> page = this.jobManagerService.listByGraphSpaces( + graphSpaces, pageNo, pageSize, query); + page.getRecords().forEach(jobManagerService::refreshStatus); + return page; + } + + private List<JobManager> visibleJobs(String query) { + Set<String> graphSpaces = this.visibleGraphSpaces(); + List<JobManager> jobs = + this.jobManagerService.listByGraphSpaces(graphSpaces); + if (StringUtils.isEmpty(query)) { + return jobs; + } + return jobs.stream() + .filter(job -> StringUtils.contains(job.getJobName(), query)) + .collect(Collectors.toList()); + } + + private Set<String> visibleGraphSpaces() { + if (this.config == null || + !this.config.get(HubbleOptions.PD_ENABLED)) { + return null; + } + HugeClient client = this.authClient(null, null); + if (this.authMode != null && this.authMode.anonymous()) { + return new LinkedHashSet<>( + this.graphSpaceAccessService.listAnonymous(client)); + } + if (this.userService.isSuperAdmin(client)) { + return null; + } + return new LinkedHashSet<>( + this.graphSpaceAccessService.listAccessible(client)); + } + + private void requireJobAccess(JobManager job) { + if (job == null) { + return; + } + this.requireGraphSpaceAccess(this.authClient(null, null), + job.getGraphSpace()); + } + + private void requireJobWrite(JobManager job) { + if (job == null) { + return; + } + this.requireGraphSpaceWrite(job.getGraphSpace()); + } + + private void requireLoadTaskAccess(LoadTask task) { + JobManager job = task.getJobId() == null ? null : + this.jobManagerService.get(task.getJobId()); + Ex.check(job != null, "job-manager.not-exist.id", + task.getJobId()); + this.requireJobAccess(job); + } + + private void requireLoadTaskWrite(LoadTask task) { + JobManager job = task.getJobId() == null ? null : + this.jobManagerService.get(task.getJobId()); + Ex.check(job != null, "job-manager.not-exist.id", + task.getJobId()); + this.requireJobWrite(job); + } + /** * Same format-whitelist check as * FileUploadController#checkFileValid diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/load/FileMappingController.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/load/FileMappingController.java index 39674b59e..43381c508 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/load/FileMappingController.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/load/FileMappingController.java @@ -112,6 +112,7 @@ public class FileMappingController extends BaseController { @PathVariable("graph") String graph, @PathVariable("jobId") int jobId, @PathVariable("id") int id) { + this.requireGraphSpaceWrite(graphSpace); FileMapping mapping = this.service.get(graphSpace, graph, jobId, id); if (mapping == null) { throw new ExternalException("load.file-mapping.not-exist.id", id); @@ -125,6 +126,7 @@ public class FileMappingController extends BaseController { public void clear(@PathVariable("graphspace") String graphSpace, @PathVariable("graph") String graph, @PathVariable("jobId") int jobId) { + this.requireGraphSpaceWrite(graphSpace); List<FileMapping> mappings = this.service.listByJob(graphSpace, graph, jobId); Set<Integer> fileIds = new HashSet<>(); @@ -143,6 +145,7 @@ public class FileMappingController extends BaseController { @PathVariable("jobId") int jobId, @PathVariable("id") int id, @RequestBody FileSetting newEntity) { + this.requireGraphSpaceWrite(graphSpace); Ex.check(!StringUtils.isEmpty(newEntity.getDelimiter()), "load.file-mapping.file-setting.delimiter-cannot-be-empty"); Ex.check(!StringUtils.isEmpty(newEntity.getCharset()), @@ -175,6 +178,7 @@ public class FileMappingController extends BaseController { @PathVariable("jobId") int jobId, @PathVariable("id") int id, @RequestBody VertexMapping newEntity) { + this.requireGraphSpaceWrite(graphSpace); FileMapping mapping = this.service.get(graphSpace, graph, jobId, id); if (mapping == null) { throw new ExternalException("load.file-mapping.not-exist.id", id); @@ -195,6 +199,7 @@ public class FileMappingController extends BaseController { @PathVariable("id") int id, @PathVariable("vmid") String vmId, @RequestBody VertexMapping newEntity) { + this.requireGraphSpaceWrite(graphSpace); FileMapping mapping = this.service.get(graphSpace, graph, jobId, id); if (mapping == null) { throw new ExternalException("load.file-mapping.not-exist.id", id); @@ -221,6 +226,7 @@ public class FileMappingController extends BaseController { @PathVariable("jobId") int jobId, @PathVariable("id") int id, @PathVariable("vmid") String vmid) { + this.requireGraphSpaceWrite(graphSpace); FileMapping mapping = this.service.get(graphSpace, graph, jobId, id); if (mapping == null) { throw new ExternalException("load.file-mapping.not-exist.id", id); @@ -242,6 +248,7 @@ public class FileMappingController extends BaseController { @PathVariable("jobId") int jobId, @PathVariable("id") int id, @RequestBody EdgeMapping newEntity) { + this.requireGraphSpaceWrite(graphSpace); FileMapping mapping = this.service.get(graphSpace, graph, jobId, id); if (mapping == null) { throw new ExternalException("load.file-mapping.not-exist.id", id); @@ -262,6 +269,7 @@ public class FileMappingController extends BaseController { @PathVariable("id") int id, @PathVariable("emid") String emId, @RequestBody EdgeMapping newEntity) { + this.requireGraphSpaceWrite(graphSpace); FileMapping mapping = this.service.get(graphSpace, graph, jobId, id); if (mapping == null) { throw new ExternalException("load.file-mapping.not-exist.id", id); @@ -288,6 +296,7 @@ public class FileMappingController extends BaseController { @PathVariable("jobId") int jobId, @PathVariable("id") int id, @PathVariable("emid") String emid) { + this.requireGraphSpaceWrite(graphSpace); FileMapping mapping = this.service.get(graphSpace, graph, jobId, id); if (mapping == null) { throw new ExternalException("load.file-mapping.not-exist.id", id); @@ -312,6 +321,7 @@ public class FileMappingController extends BaseController { @PathVariable("graph") String graph, @PathVariable("jobId") int jobId, @RequestBody LoadParameter newEntity) { + this.requireGraphSpaceWrite(graphSpace); this.checkLoadParameter(newEntity); List<FileMapping> mappings = this.service.listByJob(graphSpace, graph, jobId); @@ -327,6 +337,7 @@ public class FileMappingController extends BaseController { public JobManager nextStep(@PathVariable("graphspace") String graphSpace, @PathVariable("graph") String graph, @PathVariable("jobId") int jobId) { + this.requireGraphSpaceWrite(graphSpace); JobManager jobEntity = this.jobService.get(graphSpace, graph, jobId); Ex.check(jobEntity != null, "job-manager.not-exist.id", jobId); Ex.check(jobEntity.getJobStatus() == JobStatus.MAPPING, diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/load/FileUploadController.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/load/FileUploadController.java index 99c36da84..cbbe7cbc6 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/load/FileUploadController.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/load/FileUploadController.java @@ -87,6 +87,7 @@ public class FileUploadController extends BaseController { @PathVariable("jobId") int jobId, @RequestParam("names") List<String> fileNames) { + this.requireGraphSpaceWrite(graphSpace); Ex.check(this.jobService.get(graphSpace, graph, jobId) != null, "job-manager.not-exist.id", jobId); Ex.check(CollectionUtil.allUnique(fileNames), @@ -126,6 +127,7 @@ public class FileUploadController extends BaseController { @RequestParam("token") String token, @RequestParam("total") int total, @RequestParam("index") int index) { + this.requireGraphSpaceWrite(graphSpace); this.checkTotalAndIndexValid(total, index); this.checkFileNameValid(fileName); this.checkFileNameMatchToken(fileName, token); @@ -253,6 +255,7 @@ public class FileUploadController extends BaseController { @PathVariable("jobId") int jobId, @RequestParam("name") String fileName, @RequestParam("token") String token) { + this.requireGraphSpaceWrite(graphSpace); this.checkFileNameValid(fileName); JobManager jobEntity = this.jobService.get(graphSpace, graph, jobId); Ex.check(jobEntity != null, "job-manager.not-exist.id", jobId); @@ -286,6 +289,7 @@ public class FileUploadController extends BaseController { public JobManager nextStep(@PathVariable("graphspace") String graphSpace, @PathVariable("graph") String graph, @PathVariable("jobId") int jobId) { + this.requireGraphSpaceWrite(graphSpace); JobManager jobEntity = this.jobService.get(graphSpace, graph, jobId); Ex.check(jobEntity != null, "job-manager.not-exist.id", jobId); Ex.check(jobEntity.getJobStatus() == JobStatus.UPLOADING, diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/load/JobManagerController.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/load/JobManagerController.java index f3c58a3e5..a8fb2e731 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/load/JobManagerController.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/load/JobManagerController.java @@ -23,6 +23,7 @@ import lombok.extern.log4j.Log4j2; import org.apache.commons.lang3.StringUtils; import org.apache.hugegraph.common.Constant; import org.apache.hugegraph.common.Response; +import org.apache.hugegraph.controller.BaseController; import org.apache.hugegraph.entity.enums.JobStatus; import org.apache.hugegraph.entity.enums.LoadStatus; import org.apache.hugegraph.entity.load.FileMapping; @@ -54,7 +55,7 @@ import java.util.List; @RestController @RequestMapping(Constant.API_VERSION + "graphspaces/{graphspace}/graphs" + "/{graph}/job-manager") -public class JobManagerController { +public class JobManagerController extends BaseController { private static final int LIMIT = 500; @@ -73,6 +74,7 @@ public class JobManagerController { public JobManager create(@PathVariable("graphspace") String graphSpace, @PathVariable("graph") String graph, @RequestBody JobManager entity) { + this.requireGraphSpaceWrite(graphSpace); synchronized (this.service) { Ex.check(!StringUtils.isEmpty(entity.getJobName()), "common.param.cannot-be-null-or-empty", "job_name"); @@ -111,6 +113,7 @@ public class JobManagerController { public void delete(@PathVariable("graphspace") String graphSpace, @PathVariable("graph") String graph, @PathVariable("id") int id) { + this.requireGraphSpaceWrite(graphSpace); this.service.deleteJob(graphSpace, graph, id); } @@ -155,6 +158,7 @@ public class JobManagerController { @PathVariable("graph") String graph, @PathVariable("id") int id, @RequestBody JobManager newEntity) { + this.requireGraphSpaceWrite(graphSpace); Ex.check(!StringUtils.isEmpty(newEntity.getJobName()), "common.param.cannot-be-null-or-empty", "job_name"); Ex.check(newEntity.getJobName().length() <= 48, diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/load/LoadTaskController.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/load/LoadTaskController.java index 66f534c18..5b45e7800 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/load/LoadTaskController.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/load/LoadTaskController.java @@ -113,6 +113,7 @@ public class LoadTaskController extends BaseController { @PathVariable("graph") String graph, @PathVariable("jobId") int jobId, @RequestBody LoadTask entity) { + this.requireGraphSpaceWrite(graphSpace); JobManager jobEntity = this.jobService.get(graphSpace, graph, jobId); Ex.check(jobEntity != null, "job-manager.not-exist.id", jobId); Ex.check(jobEntity.getJobStatus() == JobStatus.SETTING, @@ -133,6 +134,7 @@ public class LoadTaskController extends BaseController { @PathVariable("graph") String graph, @PathVariable("jobId") int jobId, @PathVariable("id") int id) { + this.requireGraphSpaceWrite(graphSpace); LoadTask task = this.service.get(graphSpace, graph, jobId, id); if (task == null) { throw new ExternalException("load.task.not-exist.id", id); @@ -152,6 +154,7 @@ public class LoadTaskController extends BaseController { @PathVariable("jobId") int jobId, @RequestParam("file_mapping_ids") List<Integer> fileIds) { + this.requireGraphSpaceWrite(graphSpace); GraphConnection connection = new GraphConnection(); connection.setCluster(config.get(HubbleOptions.PD_CLUSTER)); @@ -205,6 +208,7 @@ public class LoadTaskController extends BaseController { @PathVariable("graph") String graph, @PathVariable("jobId") int jobId, @RequestParam("task_id") int taskId) { + this.requireGraphSpaceWrite(graphSpace); JobManager jobEntity = this.jobService.get(graphSpace, graph, jobId); Ex.check(jobEntity != null, "job-manager.not-exist.id", jobId); Ex.check(this.service.get(graphSpace, graph, jobId, taskId) != null, @@ -225,6 +229,7 @@ public class LoadTaskController extends BaseController { @PathVariable("graph") String graph, @PathVariable("jobId") int jobId, @RequestParam("task_id") int taskId) { + this.requireGraphSpaceWrite(graphSpace); JobManager jobEntity = this.jobService.get(graphSpace, graph, jobId); Ex.check(jobEntity != null, "job-manager.not-exist.id", jobId); Ex.check(this.service.get(graphSpace, graph, jobId, taskId) != null, @@ -245,6 +250,7 @@ public class LoadTaskController extends BaseController { @PathVariable("graph") String graph, @PathVariable("jobId") int jobId, @RequestParam("task_id") int taskId) { + this.requireGraphSpaceWrite(graphSpace); JobManager jobEntity = this.jobService.get(graphSpace, graph, jobId); Ex.check(jobEntity != null, "job-manager.not-exist.id", jobId); Ex.check(this.service.get(graphSpace, graph, jobId, taskId) != null, @@ -265,6 +271,7 @@ public class LoadTaskController extends BaseController { @PathVariable("graph") String graph, @PathVariable("jobId") int jobId, @RequestParam("task_id") int taskId) { + this.requireGraphSpaceWrite(graphSpace); JobManager jobEntity = this.jobService.get(graphSpace, graph, jobId); Ex.check(jobEntity != null, "job-manager.not-exist.id", jobId); Ex.check(this.service.get(graphSpace, graph, jobId, taskId) != null, diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/query/ExecuteHistoryController.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/query/ExecuteHistoryController.java index b7f1fe24c..945297a38 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/query/ExecuteHistoryController.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/query/ExecuteHistoryController.java @@ -65,14 +65,19 @@ public class ExecuteHistoryController extends GremlinController { @PathVariable("graph") String graph, @PathVariable("id") int id) { HugeClient client = this.authClient(graphSpace, graph); - return this.service.get(client, id); + ExecuteHistory history = this.service.get(client, id); + if (history == null) { + throw new ExternalException("execute-history.not-exist.id", id); + } + return history; } @DeleteMapping("{id}") public ExecuteHistory delete(@PathVariable("graphspace") String graphSpace, @PathVariable("graph") String graph, @PathVariable("id") int id) { - HugeClient client = this.authClient(graphSpace, graph); + HugeClient client = this.requireGraphSpaceWrite(graphSpace); + client.assignGraph(graphSpace, graph); ExecuteHistory oldEntity = this.service.get(client, id); if (oldEntity == null) { throw new ExternalException("execute-history.not-exist.id", id); diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/query/GremlinCollectionController.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/query/GremlinCollectionController.java index 8f66c8590..922b131c1 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/query/GremlinCollectionController.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/query/GremlinCollectionController.java @@ -105,14 +105,18 @@ public class GremlinCollectionController extends GremlinController { } @GetMapping("{id}") - public GremlinCollection get(@PathVariable("id") int id) { - return this.service.get(id); + public GremlinCollection get( + @PathVariable("graphspace") String graphSpace, + @PathVariable("graph") String graph, + @PathVariable("id") int id) { + return this.service.get(graphSpace, graph, id); } @PostMapping public GremlinCollection create(@PathVariable("graphspace") String graphSpace, @PathVariable("graph") String graph, @RequestBody GremlinCollection newEntity) { + this.requireGraphSpaceWrite(graphSpace); this.checkParamsValid(newEntity, true); newEntity.setGraphSpace(graphSpace); newEntity.setGraph(graph); @@ -128,29 +132,39 @@ public class GremlinCollectionController extends GremlinController { } @PutMapping("{id}") - public GremlinCollection update(@PathVariable("id") int id, + public GremlinCollection update( + @PathVariable("graphspace") String graphSpace, + @PathVariable("graph") String graph, + @PathVariable("id") int id, @RequestBody GremlinCollection newEntity) { + this.requireGraphSpaceWrite(graphSpace); this.checkIdSameAsBody(id, newEntity); this.checkParamsValid(newEntity, false); - GremlinCollection oldEntity = this.service.get(id); + GremlinCollection oldEntity = this.service.get(graphSpace, graph, id); if (oldEntity == null) { throw new ExternalException("gremlin-collection.not-exist.id", id); } GremlinCollection entity = this.mergeEntity(oldEntity, newEntity); + entity.setGraphSpace(graphSpace); + entity.setGraph(graph); this.checkEntityUnique(entity, false); - this.service.update(entity); + this.service.update(graphSpace, graph, entity); return entity; } @DeleteMapping("{id}") - public GremlinCollection delete(@PathVariable("id") int id) { - GremlinCollection oldEntity = this.service.get(id); + public GremlinCollection delete( + @PathVariable("graphspace") String graphSpace, + @PathVariable("graph") String graph, + @PathVariable("id") int id) { + this.requireGraphSpaceWrite(graphSpace); + GremlinCollection oldEntity = this.service.get(graphSpace, graph, id); if (oldEntity == null) { throw new ExternalException("gremlin-collection.not-exist.id", id); } - this.service.remove(id); + this.service.remove(graphSpace, graph, id); return oldEntity; } diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/space/GraphSpaceController.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/space/GraphSpaceController.java index e3a8e2d6a..60fb08a8a 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/space/GraphSpaceController.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/space/GraphSpaceController.java @@ -81,10 +81,14 @@ public class GraphSpaceController extends BaseController { } HugeClient client = this.authClient(null, null); - List<String> graphSpaces = - this.authMode != null && this.authMode.anonymous() ? - this.graphSpaceService.listAnonymous(client) : - this.graphSpaceService.listAll(client); + List<String> graphSpaces; + if (this.authMode != null && this.authMode.anonymous()) { + graphSpaces = this.graphSpaceService.listAnonymous(client); + } else if (this.userService.isSuperAdmin(client)) { + graphSpaces = this.graphSpaceService.listAll(client); + } else { + graphSpaces = this.graphSpaceService.listAccessible(client); + } return ImmutableMap.of("graphspaces", graphSpaces); } @@ -133,10 +137,16 @@ public class GraphSpaceController extends BaseController { return ImmutableMap.of("auth", false); } HugeClient client = this.authClient(null, null); - boolean isAuth = - this.authMode != null && this.authMode.anonymous() ? - this.graphSpaceService.isAuthForAnonymous(client, graphSpace) : - this.graphSpaceService.isAuth(client, graphSpace); + boolean isAuth; + if (this.authMode != null && this.authMode.anonymous()) { + isAuth = this.graphSpaceService.isAuthForAnonymous(client, + graphSpace); + } else if (this.userService.isSuperAdmin(client)) { + isAuth = this.graphSpaceService.isAuth(client, graphSpace); + } else { + isAuth = this.graphSpaceService.isAuthForAccessible(client, + graphSpace); + } return ImmutableMap.of("auth", isAuth); } @@ -153,9 +163,12 @@ public class GraphSpaceController extends BaseController { if (this.authMode != null && this.authMode.anonymous()) { return this.graphSpaceService.getAnonymous(client, graphspace); } - // Get GraphSpace Info - return graphSpaceService.toView( - graphSpaceService.getWithAdmins(client, graphspace)); + GraphSpaceEntity entity = this.userService.isSuperAdmin(client) ? + graphSpaceService.getWithAdmins(client, + graphspace) : + graphSpaceService.getAccessibleWithAdmins( + client, graphspace); + return graphSpaceService.toView(entity); } @PostMapping diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/space/VermeerController.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/space/VermeerController.java index 27508d836..4206a8fec 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/space/VermeerController.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/controller/space/VermeerController.java @@ -67,7 +67,7 @@ public class VermeerController extends BaseController { String graphspace = body.graphspace; String graph = body.graph; String vGraph = vermeerService.convert2VG(graphspace, graph); - HugeClient client = this.authClient(null, null); + HugeClient client = this.authClient(graphspace, graph); Map<String, Object> graphInfo = HubbleUtil.uncheckedCast( client.vermeer().getGraphInfoByName(vGraph).get("graph")); diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/handler/CustomInterceptor.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/handler/CustomInterceptor.java index 09e88c3ec..ba7abc40c 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/handler/CustomInterceptor.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/handler/CustomInterceptor.java @@ -29,6 +29,7 @@ import javax.servlet.http.HttpSession; //import org.apache.hugegraph.license.LicenseVerifier; // TODO C Remove Licence import org.apache.hugegraph.service.HugeClientPoolService; import org.apache.hugegraph.service.auth.AuthModeService; +import org.apache.hugegraph.service.space.GraphSpaceService; //import org.apache.hugegraph.service.license.LicenseService;// TODO C Remove Licence import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @@ -36,8 +37,10 @@ import org.springframework.util.StringUtils; import org.springframework.web.servlet.handler.HandlerInterceptorAdapter; import org.apache.hugegraph.common.Constant; +import org.apache.hugegraph.config.HugeConfig; import org.apache.hugegraph.driver.HugeClient; import org.apache.hugegraph.exception.ExternalException; +import org.apache.hugegraph.options.HubbleOptions; import org.apache.hugegraph.util.PageUtil; import lombok.extern.log4j.Log4j2; @@ -52,6 +55,10 @@ public class CustomInterceptor extends HandlerInterceptorAdapter { protected HugeClientPoolService hugeClientPoolService; @Autowired protected AuthModeService authMode; + @Autowired + protected HugeConfig config; + @Autowired + protected GraphSpaceService graphSpaceService; private static final Pattern CHECK_API_PATTERN = Pattern.compile(".*/graph-connections/\\d+/.+"); @@ -134,7 +141,9 @@ public class CustomInterceptor extends HandlerInterceptorAdapter { String[] scope = this.requestScope(uri); String graphSpace = scope[0]; String graph = scope[1]; - if (this.authMode != null && this.authMode.anonymous()) { + boolean anonymous = this.authMode != null && + this.authMode.anonymous(); + if (anonymous) { client = unauthClient(graphSpace, graph); } else if (!this.hasAuthSession(request)) { return; @@ -143,11 +152,35 @@ public class CustomInterceptor extends HandlerInterceptorAdapter { (String) request.getSession().getAttribute(Constant.TOKEN_KEY); client = this.authClient(graphSpace, graph, token); } + this.requireGraphSpaceAccess(client, graphSpace, anonymous); + request.setAttribute(Constant.GRAPHSPACE_ACCESS_KEY, graphSpace); } request.setAttribute("hugeClient", client); } + protected void requireGraphSpaceAccess(HugeClient client, + String graphSpace, + boolean anonymous) { + if (graphSpace == null || this.config == null || + !this.config.get(HubbleOptions.PD_ENABLED)) { + return; + } + try { + if (anonymous) { + this.graphSpaceService.requirePublicSpace(client, graphSpace); + } else { + this.graphSpaceService.requireAccessibleSpace(client, + graphSpace); + } + } catch (RuntimeException e) { + if (client != null) { + client.close(); + } + throw e; + } + } + private boolean isLoginRequest(String uri) { return (Constant.API_VERSION + "auth/login").equals(uri) || uri.endsWith("/auth/login"); @@ -192,8 +225,13 @@ public class CustomInterceptor extends HandlerInterceptorAdapter { String[] parts = uri.split("/"); for (int i = 0; i < parts.length; i++) { if ("graphspaces".equals(parts[i]) && i < parts.length - 1) { - graphSpace = parts[i + 1]; - graphSpace = decodeSegment(graphSpace); + String candidate = parts[i + 1]; + boolean collectionAction = i + 1 == parts.length - 1 && + ("list".equals(candidate) || + "builtin".equals(candidate)); + if (!collectionAction) { + graphSpace = decodeSegment(candidate); + } } if ("graphs".equals(parts[i]) && i < parts.length - 1) { String candidate = parts[i + 1]; diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/handler/LoginInterceptor.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/handler/LoginInterceptor.java index ae3baee1e..74b62e38b 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/handler/LoginInterceptor.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/handler/LoginInterceptor.java @@ -90,7 +90,6 @@ public class LoginInterceptor extends HandlerInterceptorAdapter { return false; } String scopedAuth = apiPath.substring(0, graphSpaceEnd) + "/auth"; - return apiPath.equals(scopedAuth) || - apiPath.startsWith(scopedAuth + "/"); + return apiPath.startsWith(scopedAuth + "/"); } } diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/auth/GraphSpaceUserService.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/auth/GraphSpaceUserService.java index 325b6e382..93445da82 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/auth/GraphSpaceUserService.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/auth/GraphSpaceUserService.java @@ -470,7 +470,8 @@ public class GraphSpaceUserService extends AuthService { public boolean hasGraphSpaceAccess(HugeClient client, String graphSpace, String username) { if (!client.supportsDefaultRole()) { - return false; + return client.auth().listSpaceMember(graphSpace) + .contains(username); } return client.graphSpace().checkDefaultRole( graphSpace, username, "analyst") || diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/auth/UserService.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/auth/UserService.java index 0d44836db..70b114e42 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/auth/UserService.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/auth/UserService.java @@ -24,6 +24,7 @@ import java.util.ArrayList; import java.util.Comparator; import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -235,15 +236,8 @@ public class UserService extends AuthService { } User newUser = client.auth().createUser(user); - List<String> attemptedAdminSpaces = new ArrayList<>(); boolean superAdminAttempted = false; try { - if (!permissionPresets && ue.getAdminSpaces() != null) { - for (String graphspace : ue.getAdminSpaces()) { - attemptedAdminSpaces.add(graphspace); - client.auth().addSpaceAdmin(ue.getName(), graphspace); - } - } if (permissionPresets) { this.graphSpaceUserService .applyPermissionPresetsForNewAccount( @@ -251,13 +245,12 @@ public class UserService extends AuthService { ue.getGraphspacePermissions(), ue.getPermissionPreset()); } - if (newUser != null && ue.isSuperadmin()) { + if (permissionPresets && newUser != null && ue.isSuperadmin()) { superAdminAttempted = true; client.auth().addSuperAdmin(ue.getName()); } } catch (RuntimeException error) { this.rollbackNewAccount(client, newUser, ue.getName(), - attemptedAdminSpaces, superAdminAttempted, error); throw error; } @@ -265,19 +258,12 @@ public class UserService extends AuthService { private void rollbackNewAccount(HugeClient client, User user, String username, - List<String> attemptedAdminSpaces, boolean superAdminAttempted, RuntimeException failure) { if (superAdminAttempted) { this.suppressRollback( () -> client.auth().delSuperAdmin(username), failure); } - for (int i = attemptedAdminSpaces.size() - 1; i >= 0; i--) { - String graphSpace = attemptedAdminSpaces.get(i); - this.suppressRollback( - () -> client.auth().delSpaceAdmin(username, graphSpace), - failure); - } if (user != null) { this.suppressRollback( () -> client.auth().deleteUser(user.id()), failure); @@ -590,6 +576,9 @@ public class UserService extends AuthService { boolean permissionMutation = userEntity.getPermissionPreset() != null; this.validatePermissionMutation(hugeClient, userEntity, false, permissionPresets); + if (isPdEnabled() && !permissionPresets) { + this.validateLegacyPermissionUpdate(hugeClient, userEntity); + } User user = new User(); user.setId(userEntity.getId()); user.name(userEntity.getName()); @@ -604,23 +593,30 @@ public class UserService extends AuthService { this.updateModernPermissionAccount(hugeClient, user, userEntity); return; } - if (!permissionPresets) { - updateAdminSpace(hugeClient, userEntity.getName(), - userEntity.getAdminSpaces()); + hugeClient.auth().updateUser(user); + } + + private void validateLegacyPermissionUpdate(HugeClient client, + UserEntity user) { + if (user.getPermissionPreset() != null || + user.getGraphspacePermissions() != null && + !user.getGraphspacePermissions().isEmpty()) { + throw new ParameterizedException( + "auth.permission-preset.unsupported"); } - if (!permissionPresets) { - String username = userEntity.getName(); - boolean currentSuperAdmin = - isSuperAdmin(hugeClient, user.id().toString()); - if (currentSuperAdmin && !userEntity.isSuperadmin()) { - hugeClient.auth().delSuperAdmin(username); - } - if (!currentSuperAdmin && userEntity.isSuperadmin()) { - hugeClient.auth().addSuperAdmin(username); + if (user.getAdminSpaces() != null) { + List<String> current = this.listAdminSpace(client, user.getName()); + if (!new HashSet<>(current).equals( + new HashSet<>(user.getAdminSpaces()))) { + throw new ParameterizedException( + "auth.permission-preset.unsupported"); } } - - hugeClient.auth().updateUser(user); + if (user.hasSuperadmin() && + this.isSuperAdmin(client, user.getId()) != user.isSuperadmin()) { + throw new ParameterizedException( + "auth.permission-preset.unsupported"); + } } private void updateModernPermissionAccount(HugeClient client, User user, @@ -683,6 +679,15 @@ public class UserService extends AuthService { boolean create, boolean supported) { if (!supported) { + if (create && (user.isSuperadmin() || + user.getPermissionPreset() != null || + user.getAdminSpaces() != null && + !user.getAdminSpaces().isEmpty() || + user.getGraphspacePermissions() != null && + !user.getGraphspacePermissions().isEmpty())) { + throw new ParameterizedException( + "auth.permission-preset.unsupported"); + } return; } String preset = user.getPermissionPreset(); @@ -693,6 +698,11 @@ public class UserService extends AuthService { } return; } + if (!create && user.getPassword() != null && + !user.getPassword().isEmpty()) { + throw new ParameterizedException( + "Update password and permissions separately"); + } boolean superAdminPreset = "SUPER_ADMIN".equals(preset); if (superAdminPreset != user.isSuperadmin()) { throw new ParameterizedException( @@ -779,22 +789,13 @@ public class UserService extends AuthService { if (adminspaces == null || !isPdEnabled()) { return; } + if (!hugeClient.supportsDefaultRole()) { + throw new ParameterizedException( + "auth.permission-preset.unsupported"); + } List<String> oldadminspaces = listAdminSpace(hugeClient, username); User account = hugeClient.findUserByName(username); E.checkNotNull(account, "User"); - if (!hugeClient.supportsDefaultRole()) { - for (String adminspace : adminspaces) { - if (!oldadminspaces.contains(adminspace)) { - hugeClient.auth().addSpaceAdmin(username, adminspace); - } - } - for (String oldadminspace : oldadminspaces) { - if (!adminspaces.contains(oldadminspace)) { - hugeClient.auth().delSpaceAdmin(username, oldadminspace); - } - } - return; - } for (String adminspace : adminspaces) { if (!oldadminspaces.contains(adminspace)) { this.graphSpaceUserService.applySpacePreset(hugeClient, adminspace, account.id().toString(), diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/load/JobManagerService.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/load/JobManagerService.java index ecbf666ed..e2578aa27 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/load/JobManagerService.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/load/JobManagerService.java @@ -19,6 +19,8 @@ package org.apache.hugegraph.service.load; import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; import java.util.Date; import java.util.List; @@ -119,6 +121,38 @@ public class JobManagerService { return this.mapper.selectList(null); } + public List<JobManager> listByGraphSpaces( + Collection<String> graphSpaces) { + if (graphSpaces != null && graphSpaces.isEmpty()) { + return Collections.emptyList(); + } + QueryWrapper<JobManager> query = Wrappers.query(); + if (graphSpaces != null) { + query.in("graphspace", graphSpaces); + } + query.orderByDesc("create_time"); + return this.mapper.selectList(query); + } + + public IPage<JobManager> listByGraphSpaces( + Collection<String> graphSpaces, int pageNo, int pageSize, + String content) { + Page<JobManager> page = new Page<>(pageNo, + PageUtil.boundedSize(pageSize)); + if (graphSpaces != null && graphSpaces.isEmpty()) { + return page; + } + QueryWrapper<JobManager> query = Wrappers.query(); + if (graphSpaces != null) { + query.in("graphspace", graphSpaces); + } + if (content != null && !content.isEmpty()) { + query.like("job_name", content); + } + query.orderByDesc("create_time"); + return this.mapper.selectPage(page, query); + } + public IPage<JobManager> listAll(int pageNo, int pageSize, String content) { QueryWrapper<JobManager> query = Wrappers.query(); if (content != null && !content.isEmpty()) { diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/query/ExecuteHistoryService.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/query/ExecuteHistoryService.java index a56e85188..1af1a9bbd 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/query/ExecuteHistoryService.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/query/ExecuteHistoryService.java @@ -130,7 +130,14 @@ public class ExecuteHistoryService { } public ExecuteHistory get(HugeClient client, int id) { - ExecuteHistory history = this.mapper.selectById(id); + QueryWrapper<ExecuteHistory> query = Wrappers.query(); + query.eq("id", id) + .eq("graphspace", client.getGraphSpaceName()) + .eq("graph", client.getGraphName()); + ExecuteHistory history = this.mapper.selectOne(query); + if (history == null) { + return null; + } if (history.getType().equals(ExecuteType.GREMLIN_ASYNC)) { try { Task task = client.task().get(history.getAsyncId()); @@ -160,11 +167,18 @@ public class ExecuteHistoryService { @Transactional(isolation = Isolation.READ_COMMITTED) public void remove(HugeClient client, int id) { - ExecuteHistory history = this.mapper.selectById(id); + ExecuteHistory history = this.get(client, id); + if (history == null) { + return; + } if (history.getType().equals(ExecuteType.GREMLIN_ASYNC)) { client.task().delete(history.getAsyncId()); } - if (this.mapper.deleteById(id) != 1) { + QueryWrapper<ExecuteHistory> query = Wrappers.query(); + query.eq("id", id) + .eq("graphspace", client.getGraphSpaceName()) + .eq("graph", client.getGraphName()); + if (this.mapper.delete(query) != 1) { throw new InternalException("entity.delete.failed", history); } } diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/query/GremlinCollectionService.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/query/GremlinCollectionService.java index 963c77e5e..a6b016679 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/query/GremlinCollectionService.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/query/GremlinCollectionService.java @@ -111,8 +111,12 @@ public class GremlinCollectionService { } } - public GremlinCollection get(int id) { - return this.mapper.selectById(id); + public GremlinCollection get(String graphSpace, String graph, int id) { + QueryWrapper<GremlinCollection> query = Wrappers.query(); + query.eq("id", id) + .eq("graphspace", graphSpace) + .eq("graph", graph); + return this.mapper.selectOne(query); } public GremlinCollection getByName(String graphSpace, String graph, @@ -137,15 +141,24 @@ public class GremlinCollectionService { } @Transactional(isolation = Isolation.READ_COMMITTED) - public void update(GremlinCollection collection) { - if (this.mapper.updateById(collection) != 1) { + public void update(String graphSpace, String graph, + GremlinCollection collection) { + QueryWrapper<GremlinCollection> query = Wrappers.query(); + query.eq("id", collection.getId()) + .eq("graphspace", graphSpace) + .eq("graph", graph); + if (this.mapper.update(collection, query) != 1) { throw new InternalException("entity.update.failed", collection); } } @Transactional(isolation = Isolation.READ_COMMITTED) - public void remove(int id) { - if (this.mapper.deleteById(id) != 1) { + public void remove(String graphSpace, String graph, int id) { + QueryWrapper<GremlinCollection> query = Wrappers.query(); + query.eq("id", id) + .eq("graphspace", graphSpace) + .eq("graph", graph); + if (this.mapper.delete(query) != 1) { throw new InternalException("entity.delete.failed", id); } } diff --git a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/space/GraphSpaceService.java b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/space/GraphSpaceService.java index 2b5afc2dc..498c56541 100644 --- a/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/space/GraphSpaceService.java +++ b/hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/space/GraphSpaceService.java @@ -186,9 +186,7 @@ public class GraphSpaceService { space.getNickname().contains(prefix))) .filter(space -> space.getCreateTime() == null || space.getCreateTime().compareTo(after) > 0) - .filter(space -> !space.isAuth() || - client.auth().isSpaceAdmin(space.getName()) || - hasCurrentUserAccess(client, space.getName())) + .filter(space -> canCurrentUserAccess(client, space)) .collect(Collectors.toList()); Collections.sort(results, (a, b) -> new BuiltInFirst().compare( a.getName(), b.getName())); @@ -216,6 +214,14 @@ public class GraphSpaceService { return client.auth().checkDefaultRole(graphSpace, "observer"); } + private static boolean canCurrentUserAccess(HugeClient client, + GraphSpace graphSpace) { + return !graphSpace.isAuth() || + client.auth().isSuperAdmin() || + client.auth().isSpaceAdmin(graphSpace.getName()) || + hasCurrentUserAccess(client, graphSpace.getName()); + } + public List<Map<String, Object>> queryAnonymousGs(HugeClient client, String query, String createTime) { @@ -230,13 +236,19 @@ public class GraphSpaceService { .collect(Collectors.toList()); } + public List<String> listAccessible(HugeClient client) { + return queryAccessibleSpaces(client, "", "").stream() + .map(GraphSpace::getName) + .collect(Collectors.toList()); + } + public Map<String, Object> getAnonymous(HugeClient client, String graphSpace) { - return anonymousView(client, publicSpace(client, graphSpace)); + return anonymousView(client, requirePublicSpace(client, graphSpace)); } public boolean isAuthForAnonymous(HugeClient client, String graphSpace) { - return publicSpace(client, graphSpace).isAuth(); + return requirePublicSpace(client, graphSpace).isAuth(); } public IPage<Map<String, Object>> queryAnonymousGsPage( @@ -300,8 +312,17 @@ public class GraphSpaceService { return info; } - private static GraphSpace publicSpace(HugeClient client, - String graphSpace) { + public GraphSpace requirePublicSpace(HugeClient client, + String graphSpace) { + GraphSpace space = graphSpaceOrUnavailable(client, graphSpace); + if (space.isAuth()) { + throw unavailableGraphSpace(); + } + return space; + } + + private static GraphSpace graphSpaceOrUnavailable(HugeClient client, + String graphSpace) { GraphSpace space; try { space = client.graphSpace().getGraphSpace(graphSpace); @@ -312,7 +333,7 @@ public class GraphSpaceService { } throw e; } - if (space == null || space.isAuth()) { + if (space == null) { throw unavailableGraphSpace(); } return space; @@ -464,6 +485,11 @@ public class GraphSpaceService { return space.isAuth(); } + public boolean isAuthForAccessible(HugeClient client, + String graphSpace) { + return requireAccessibleSpace(client, graphSpace).isAuth(); + } + public List<String> listAll(HugeClient client) { List<String> result = client.graphSpace().listGraphSpace().stream() .collect(Collectors.toList()); @@ -488,17 +514,34 @@ public class GraphSpaceService { throw new InternalException("graphspace.get.{} Not Exits", graphspace); } + return this.withAdmins(authClient, space); + } - GraphSpaceEntity graphSpaceEntity - = GraphSpaceEntity.fromGraphSpace(space); + public GraphSpaceEntity getAccessibleWithAdmins(HugeClient client, + String graphSpace) { + return this.withAdmins(client, requireAccessibleSpace(client, + graphSpace)); + } - if (authClient.auth().isSuperAdmin()) { - graphSpaceEntity.graphspaceAdmin = - userService.listGraphSpaceAdmin(authClient, graphspace); + private GraphSpaceEntity withAdmins(HugeClient client, + GraphSpace graphSpace) { + GraphSpaceEntity entity = GraphSpaceEntity.fromGraphSpace(graphSpace); + if (client.auth().isSuperAdmin()) { + String name = graphSpace.getName(); + entity.graphspaceAdmin = + userService.listGraphSpaceAdmin(client, name); } - graphSpaceEntity.setStatistic(evCount(authClient, graphspace)); + entity.setStatistic(evCount(client, graphSpace.getName())); + return entity; + } - return graphSpaceEntity; + public GraphSpace requireAccessibleSpace(HugeClient client, + String graphSpace) { + GraphSpace space = graphSpaceOrUnavailable(client, graphSpace); + if (!canCurrentUserAccess(client, space)) { + throw unavailableGraphSpace(); + } + return space; } public void delete(HugeClient authClient, String graphspace) { diff --git a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/controller/ingest/IngestControllerTest.java b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/controller/ingest/IngestControllerTest.java index 639bc341b..e1c9841db 100644 --- a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/controller/ingest/IngestControllerTest.java +++ b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/controller/ingest/IngestControllerTest.java @@ -51,10 +51,12 @@ import org.apache.hugegraph.entity.load.JobManager; import org.apache.hugegraph.entity.load.LoadTask; import org.apache.hugegraph.exception.ExternalException; import org.apache.hugegraph.options.HubbleOptions; +import org.apache.hugegraph.service.auth.AuthModeService; import org.apache.hugegraph.service.load.DatasourceService; import org.apache.hugegraph.service.load.FileMappingService; import org.apache.hugegraph.service.load.JobManagerService; import org.apache.hugegraph.service.load.LoadTaskService; +import org.apache.hugegraph.service.space.GraphSpaceService; import org.apache.hugegraph.testutil.Assert; public class IngestControllerTest { @@ -335,7 +337,15 @@ public class IngestControllerTest { throws Exception { TestIngestController controller = new TestIngestController(); LoadTaskService loadTaskService = Mockito.mock(LoadTaskService.class); + JobManagerService jobManagerService = + Mockito.mock(JobManagerService.class); this.setField(controller, "loadTaskService", loadTaskService); + this.setField(controller, "jobManagerService", jobManagerService); + Mockito.when(jobManagerService.get(7)) + .thenReturn(JobManager.builder() + .graphSpace("DEFAULT") + .graph("hugegraph") + .build()); LoadTask task = LoadTask.builder() .id(9) @@ -362,6 +372,99 @@ public class IngestControllerTest { Assert.assertEquals(63L, metrics.totalTime); } + @Test + public void testTaskListFiltersProtectedGraphSpacesInAnonymousMode() + throws Exception { + TestIngestController controller = new TestIngestController(); + HugeConfig config = Mockito.mock(HugeConfig.class); + Mockito.when(config.get(HubbleOptions.AUTH_ENABLED)) + .thenReturn(false); + Mockito.when(config.get(HubbleOptions.PD_ENABLED)) + .thenReturn(true); + GraphSpaceService graphSpaces = Mockito.mock(GraphSpaceService.class); + JobManagerService jobs = Mockito.mock(JobManagerService.class); + LoadTaskService loadTasks = Mockito.mock(LoadTaskService.class); + JobManager visible = JobManager.builder() + .id(1) + .jobName("visible") + .graphSpace("public") + .graph("graph") + .jobStatus(JobStatus.DEFAULT) + .build(); + com.baomidou.mybatisplus.extension.plugins.pagination.Page< + JobManager> visiblePage = + new com.baomidou.mybatisplus.extension.plugins.pagination.Page<>( + 1, 10, 1); + visiblePage.setRecords(Collections.singletonList(visible)); + Mockito.when(jobs.listByGraphSpaces( + Collections.singleton("public"), 1, 10, "")) + .thenReturn(visiblePage); + Mockito.when(graphSpaces.listAnonymous(Mockito.any())) + .thenReturn(Collections.singletonList("public")); + this.setField(controller, "config", config); + this.setField(controller, "authMode", new AuthModeService(config)); + this.setField(controller, "graphSpaceAccessService", graphSpaces); + this.setField(controller, "jobManagerService", jobs); + this.setField(controller, "loadTaskService", loadTasks); + + Response response = controller.taskList("", 1, 10); + + @SuppressWarnings("unchecked") + com.baomidou.mybatisplus.core.metadata.IPage< + IngestController.TaskVO> page = + (com.baomidou.mybatisplus.core.metadata.IPage< + IngestController.TaskVO>) + response.getData(); + Assert.assertEquals(1L, page.getTotal()); + @SuppressWarnings("unchecked") + Map<String, Object> option = (Map<String, Object>) + page.getRecords().get(0).ingestionOption; + Assert.assertEquals("public", option.get("graphspace")); + Mockito.verify(loadTasks).taskListByJob(1); + Mockito.verify(jobs).listByGraphSpaces( + Collections.singleton("public"), 1, 10, ""); + } + + @Test + public void testTaskDetailValidatesOwningGraphSpace() throws Exception { + TestIngestController controller = new TestIngestController(); + JobManagerService jobs = Mockito.mock(JobManagerService.class); + JobManager job = JobManager.builder() + .id(7) + .graphSpace("protected") + .graph("graph") + .build(); + Mockito.when(jobs.get(7)).thenReturn(job); + this.setField(controller, "jobManagerService", jobs); + + Response response = controller.taskDetail(7); + + Assert.assertEquals(Constant.STATUS_OK, response.getStatus()); + Assert.assertEquals("protected", controller.checkedGraphSpace); + } + + @Test + public void testJobDetailValidatesParentGraphSpace() throws Exception { + TestIngestController controller = new TestIngestController(); + JobManagerService jobs = Mockito.mock(JobManagerService.class); + LoadTaskService loadTasks = Mockito.mock(LoadTaskService.class); + LoadTask task = LoadTask.builder().id(9).jobId(7).build(); + Mockito.when(loadTasks.get(9)).thenReturn(task); + Mockito.when(jobs.get(7)) + .thenReturn(JobManager.builder() + .id(7) + .graphSpace("protected") + .graph("graph") + .build()); + this.setField(controller, "jobManagerService", jobs); + this.setField(controller, "loadTaskService", loadTasks); + + Response response = controller.jobDetail(9); + + Assert.assertEquals(Constant.STATUS_OK, response.getStatus()); + Assert.assertEquals("protected", controller.checkedGraphSpace); + } + private IngestController.IngestTaskRequest request(Path dataFile) { IngestController.IngestTaskRequest request = new IngestController.IngestTaskRequest(); @@ -474,9 +577,24 @@ public class IngestControllerTest { private static class TestIngestController extends IngestController { + private String checkedGraphSpace; + private String writeGraphSpace; + @Override protected HugeClient authClient(String graphSpace, String graph) { return Mockito.mock(HugeClient.class); } + + @Override + protected void requireGraphSpaceAccess(HugeClient client, + String graphSpace) { + this.checkedGraphSpace = graphSpace; + } + + @Override + protected HugeClient requireGraphSpaceWrite(String graphSpace) { + this.writeGraphSpace = graphSpace; + return Mockito.mock(HugeClient.class); + } } } diff --git a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/controller/space/GraphSpaceControllerTest.java b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/controller/space/GraphSpaceControllerTest.java index 7911be002..8280a659e 100644 --- a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/controller/space/GraphSpaceControllerTest.java +++ b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/controller/space/GraphSpaceControllerTest.java @@ -96,6 +96,7 @@ public class GraphSpaceControllerTest { HugeConfig config = Mockito.mock(HugeConfig.class); Mockito.when(config.get(HubbleOptions.PD_ENABLED)).thenReturn(true); controller.config = config; + ReflectionTestUtils.setField(controller, "userService", userService); ReflectionTestUtils.setField(controller, "graphSpaceService", service); @SuppressWarnings("unchecked") @@ -152,6 +153,54 @@ public class GraphSpaceControllerTest { .evCount(client, "public"); } + @Test + public void testAuthenticatedEndpointsUseOnlyAccessibleGraphSpaces() { + HugeClient client = Mockito.mock(HugeClient.class); + UserService userService = Mockito.mock(UserService.class); + GraphSpaceService graphSpaceService = Mockito.mock( + GraphSpaceService.class); + TestGraphSpaceController controller = + new TestGraphSpaceController(client); + HugeConfig config = Mockito.mock(HugeConfig.class); + Mockito.when(config.get(HubbleOptions.PD_ENABLED)).thenReturn(true); + Mockito.when(userService.isSuperAdmin(client)).thenReturn(false); + Mockito.when(graphSpaceService.listAccessible(client)) + .thenReturn(java.util.Collections.singletonList("member")); + Mockito.when(graphSpaceService.isAuthForAccessible(client, "member")) + .thenReturn(true); + GraphSpaceEntity entity = new GraphSpaceEntity(); + entity.setName("member"); + Mockito.when(graphSpaceService.getAccessibleWithAdmins(client, + "member")) + .thenReturn(entity); + Mockito.when(graphSpaceService.toView(entity)) + .thenReturn(java.util.Collections.singletonMap("name", + "member")); + controller.config = config; + ReflectionTestUtils.setField(controller, "userService", userService); + ReflectionTestUtils.setField(controller, "graphSpaceService", + graphSpaceService); + + @SuppressWarnings("unchecked") + Map<String, Object> names = (Map<String, Object>) controller.list(); + @SuppressWarnings("unchecked") + Map<String, Object> detail = + (Map<String, Object>) controller.get("member"); + @SuppressWarnings("unchecked") + Map<String, Object> auth = + (Map<String, Object>) controller.isAuth("member"); + + Assert.assertEquals(java.util.Collections.singletonList("member"), + names.get("graphspaces")); + Assert.assertEquals("member", detail.get("name")); + Assert.assertEquals(true, auth.get("auth")); + Mockito.verify(graphSpaceService, Mockito.never()).listAll(client); + Mockito.verify(graphSpaceService, Mockito.never()) + .isAuth(client, "member"); + Mockito.verify(graphSpaceService, Mockito.never()) + .getWithAdmins(client, "member"); + } + @Test public void testApplyDefaultsForOptionalResourceLimits() { GraphSpaceEntity graphSpace = new GraphSpaceEntity(); @@ -211,6 +260,25 @@ public class GraphSpaceControllerTest { Assert.assertSame(client, controller.requireGlobalManager()); } + @Test + public void testAnonymousModeCannotMutateGraphSpaces() throws Exception { + HugeClient client = Mockito.mock(HugeClient.class); + UserService userService = Mockito.mock(UserService.class); + GraphSpaceService graphSpaceService = Mockito.mock( + GraphSpaceService.class); + TestGraphSpaceController controller = controller( + client, userService, + graphSpaceService); + HugeConfig config = Mockito.mock(HugeConfig.class); + Mockito.when(config.get(HubbleOptions.AUTH_ENABLED)).thenReturn(false); + ReflectionTestUtils.setField(controller, "authMode", + new AuthModeService(config)); + + assertForbidden(() -> controller.add(new GraphSpaceEntity())); + + Mockito.verifyZeroInteractions(userService, graphSpaceService); + } + @Test public void testForbiddenGraphSpaceMutationUsesHttpAndBody403() throws Exception { diff --git a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/service/auth/GraphSpaceUserServiceTest.java b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/service/auth/GraphSpaceUserServiceTest.java index 080150e0a..6feee6e3a 100644 --- a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/service/auth/GraphSpaceUserServiceTest.java +++ b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/service/auth/GraphSpaceUserServiceTest.java @@ -175,13 +175,17 @@ public class GraphSpaceUserServiceTest { } @Test - public void testLegacyAccessDoesNotCallNewDefaultRoleApi() { + public void testLegacyAccessUsesMembershipWithoutDefaultRoleApi() { Mockito.when(this.client.supportsDefaultRole()).thenReturn(false); + Mockito.when(this.auth.listSpaceMember("team")) + .thenReturn(Collections.singletonList("alice")); + Assert.assertTrue(this.service.hasGraphSpaceAccess( + this.client, "team", "alice")); Assert.assertFalse(this.service.hasGraphSpaceAccess( - this.client, "team", "alice")); + this.client, "team", "bob")); Mockito.verifyZeroInteractions(this.graphSpace); - Mockito.verifyZeroInteractions(this.auth); + Mockito.verify(this.auth, Mockito.times(2)).listSpaceMember("team"); } @Test diff --git a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/service/space/GraphSpaceServiceTest.java b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/service/space/GraphSpaceServiceTest.java index 6c051e7ba..166aa67f7 100644 --- a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/service/space/GraphSpaceServiceTest.java +++ b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/service/space/GraphSpaceServiceTest.java @@ -152,6 +152,9 @@ public class GraphSpaceServiceTest { .toList())); Assert.assertTrue((Boolean) response.get(0).get("authed")); Assert.assertFalse((Boolean) response.get(0).get("default")); + Assert.assertEquals(java.util.Arrays.asList( + "admin", "analyst", "observer", "public"), + this.service.listAccessible(this.client)); Mockito.verify(auth, Mockito.never()) .checkDefaultRole(Mockito.anyString(), Mockito.eq("observer"), Mockito.anyString()); @@ -255,6 +258,34 @@ public class GraphSpaceServiceTest { Mockito.verifyZeroInteractions(this.graphsService); } + @Test + public void testAuthenticatedDetailRejectsUnassignedProtectedSpace() { + GraphSpaceManager manager = Mockito.mock(GraphSpaceManager.class); + AuthManager auth = Mockito.mock(AuthManager.class); + GraphSpace protectedSpace = graphSpace("protected", true, + "20260712"); + Mockito.when(this.client.graphSpace()).thenReturn(manager); + Mockito.when(this.client.auth()).thenReturn(auth); + Mockito.when(this.client.supportsDefaultRole()).thenReturn(false); + Mockito.when(manager.getGraphSpace("protected")) + .thenReturn(protectedSpace); + + try { + this.service.getAccessibleWithAdmins(this.client, "protected"); + Assert.fail("Expected protected GraphSpace to be unavailable"); + } catch (ExternalException e) { + Assert.assertEquals(404, e.status()); + } + try { + this.service.isAuthForAccessible(this.client, "protected"); + Assert.fail("Expected protected GraphSpace auth to be unavailable"); + } catch (ExternalException e) { + Assert.assertEquals(404, e.status()); + } + Mockito.verify(auth, Mockito.times(2)).isSpaceMember("protected"); + Mockito.verifyZeroInteractions(this.graphsService); + } + @Test public void testAnonymousAuthHidesMissingGraphSpaceLikeProtectedSpace() { GraphSpaceManager manager = Mockito.mock(GraphSpaceManager.class); diff --git a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/AuthSecurityTest.java b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/AuthSecurityTest.java index 1f0568c98..34594952d 100644 --- a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/AuthSecurityTest.java +++ b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/AuthSecurityTest.java @@ -82,6 +82,7 @@ import org.apache.hugegraph.service.auth.AuthContextService; import org.apache.hugegraph.service.auth.AuthModeService; import org.apache.hugegraph.service.auth.LoginAttemptGuard; import org.apache.hugegraph.service.auth.UserService; +import org.apache.hugegraph.service.space.GraphSpaceService; import org.apache.hugegraph.structure.auth.Login; import org.apache.hugegraph.structure.auth.LoginResult; @@ -210,6 +211,11 @@ public class AuthSecurityTest { interceptor.preHandle(scopedUsers, new MockHttpServletResponse(), null)); + MockHttpServletRequest scopedStatus = new MockHttpServletRequest( + "GET", "/api/v1.3/graphspaces/DEFAULT/auth"); + Assert.assertTrue(interceptor.preHandle( + scopedStatus, new MockHttpServletResponse(), null)); + MockHttpServletRequest authGraph = new MockHttpServletRequest( "GET", "/api/v1.3/graphspaces/DEFAULT/graphs/auth/schema"); Assert.assertTrue(interceptor.preHandle( @@ -233,6 +239,55 @@ public class AuthSecurityTest { Assert.assertNull(request.getAttribute("hugeClient")); } + @Test + public void testAnonymousGraphClientRejectsProtectedGraphSpace() { + MockHttpServletRequest request = new MockHttpServletRequest(); + RequestContextHolder.setRequestAttributes( + new ServletRequestAttributes(request)); + TestBaseController controller = new TestBaseController(); + HugeConfig config = Mockito.mock(HugeConfig.class); + Mockito.when(config.get(HubbleOptions.AUTH_ENABLED)).thenReturn(false); + Mockito.when(config.get(HubbleOptions.PD_ENABLED)).thenReturn(true); + GraphSpaceService spaces = Mockito.mock(GraphSpaceService.class); + HugeClient client = Mockito.mock(HugeClient.class); + Mockito.when(spaces.requirePublicSpace(client, "protected")) + .thenThrow(new ExternalException(HttpStatus.NOT_FOUND.value(), + "unavailable")); + ReflectionTestUtils.setField(controller, "config", config); + ReflectionTestUtils.setField(controller, "authMode", + new AuthModeService(config)); + ReflectionTestUtils.setField(controller, "graphSpaceAccessService", + spaces); + + try { + controller.requireSpace(client, "protected"); + Assert.fail("Expected protected GraphSpace to be unavailable"); + } catch (ExternalException e) { + Assert.assertEquals(HttpStatus.NOT_FOUND.value(), e.status()); + } + Mockito.verify(spaces).requirePublicSpace(client, "protected"); + } + + @Test + public void testBodyGraphSpaceIsValidatedWhenPathScopeDiffers() { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setAttribute(Constant.GRAPHSPACE_ACCESS_KEY, "path-space"); + RequestContextHolder.setRequestAttributes( + new ServletRequestAttributes(request)); + TestBaseController controller = new TestBaseController(); + HugeConfig config = Mockito.mock(HugeConfig.class); + Mockito.when(config.get(HubbleOptions.PD_ENABLED)).thenReturn(true); + GraphSpaceService spaces = Mockito.mock(GraphSpaceService.class); + HugeClient client = Mockito.mock(HugeClient.class); + ReflectionTestUtils.setField(controller, "config", config); + ReflectionTestUtils.setField(controller, "graphSpaceAccessService", + spaces); + + controller.requireSpace(client, "body-space"); + + Mockito.verify(spaces).requireAccessibleSpace(client, "body-space"); + } + @Test public void testOnlyBootstrapConfigBypassesLoginInterceptor() { InterceptorRegistry registry = new InterceptorRegistry(); @@ -371,6 +426,12 @@ public class AuthSecurityTest { public void testCustomInterceptorCreatesClientForAuthenticatedApi() throws Exception { TestCustomInterceptor interceptor = new TestCustomInterceptor(); + HugeConfig config = Mockito.mock(HugeConfig.class); + Mockito.when(config.get(HubbleOptions.PD_ENABLED)) + .thenReturn(true); + GraphSpaceService spaces = Mockito.mock(GraphSpaceService.class); + ReflectionTestUtils.setField(interceptor, "config", config); + ReflectionTestUtils.setField(interceptor, "graphSpaceService", spaces); MockHttpServletRequest request = new MockHttpServletRequest( "GET", "/api/v1.3/graphspaces/space1" + @@ -387,6 +448,7 @@ public class AuthSecurityTest { Assert.assertEquals("space1", interceptor.graphSpace); Assert.assertEquals("graph1", interceptor.graph); Assert.assertEquals("token", interceptor.token); + Mockito.verify(spaces).requireAccessibleSpace(null, "space1"); } @Test @@ -408,6 +470,30 @@ public class AuthSecurityTest { Assert.assertEquals(2, interceptor.authClients); } + @Test + public void testCustomInterceptorKeepsGraphSpaceCollectionActionsUnscoped() + throws Exception { + TestCustomInterceptor interceptor = new TestCustomInterceptor(); + HugeConfig config = Mockito.mock(HugeConfig.class); + Mockito.when(config.get(HubbleOptions.PD_ENABLED)) + .thenReturn(true); + GraphSpaceService spaces = Mockito.mock(GraphSpaceService.class); + ReflectionTestUtils.setField(interceptor, "config", config); + ReflectionTestUtils.setField(interceptor, "graphSpaceService", spaces); + + for (String action : new String[]{"list", "builtin"}) { + MockHttpServletRequest request = new MockHttpServletRequest( + "GET", "/api/v1.3/graphspaces/" + action); + request.getSession().setAttribute(Constant.TOKEN_KEY, "token"); + request.getSession().setAttribute(Constant.USERNAME_KEY, "admin"); + + Assert.assertTrue(interceptor.preHandle( + request, new MockHttpServletResponse(), null)); + Assert.assertNull(interceptor.graphSpace); + } + Mockito.verifyZeroInteractions(spaces); + } + @Test public void testCustomInterceptorAllowsGraphNamedLikeCollectionAction() throws Exception { @@ -429,8 +515,13 @@ public class AuthSecurityTest { HugeConfig config = Mockito.mock(HugeConfig.class); Mockito.when(config.get(HubbleOptions.AUTH_ENABLED)) .thenReturn(false); + Mockito.when(config.get(HubbleOptions.PD_ENABLED)) + .thenReturn(true); AuthModeService mode = new AuthModeService(config); ReflectionTestUtils.setField(interceptor, "authMode", mode); + ReflectionTestUtils.setField(interceptor, "config", config); + GraphSpaceService spaces = Mockito.mock(GraphSpaceService.class); + ReflectionTestUtils.setField(interceptor, "graphSpaceService", spaces); MockHttpServletRequest request = new MockHttpServletRequest( "GET", "/api/v1.3/graphspaces/SPACE/graphs/graph/schema"); @@ -438,6 +529,38 @@ public class AuthSecurityTest { Assert.assertEquals(1, interceptor.unauthClients); Assert.assertEquals("SPACE", interceptor.graphSpace); Assert.assertEquals("graph", interceptor.graph); + Mockito.verify(spaces).requirePublicSpace(null, "SPACE"); + } + + @Test + public void testAnonymousPathScopeRejectsProtectedGraphSpace() + throws Exception { + TestCustomInterceptor interceptor = new TestCustomInterceptor(); + HugeConfig config = Mockito.mock(HugeConfig.class); + Mockito.when(config.get(HubbleOptions.AUTH_ENABLED)) + .thenReturn(false); + Mockito.when(config.get(HubbleOptions.PD_ENABLED)) + .thenReturn(true); + ReflectionTestUtils.setField(interceptor, "authMode", + new AuthModeService(config)); + ReflectionTestUtils.setField(interceptor, "config", config); + GraphSpaceService spaces = Mockito.mock(GraphSpaceService.class); + Mockito.doThrow(new ExternalException(HttpStatus.NOT_FOUND.value(), + "unavailable")) + .when(spaces).requirePublicSpace(null, "protected"); + ReflectionTestUtils.setField(interceptor, "graphSpaceService", spaces); + MockHttpServletRequest request = new MockHttpServletRequest( + "POST", "/api/v1.3/graphspaces/protected/graphs/graph" + + "/job-manager/1/upload-file"); + + try { + interceptor.preHandle(request, new MockHttpServletResponse(), null); + Assert.fail("Expected protected GraphSpace to be unavailable"); + } catch (ExternalException e) { + Assert.assertEquals(HttpStatus.NOT_FOUND.value(), e.status()); + } + Assert.assertEquals(1, interceptor.unauthClients); + Mockito.verify(spaces).requirePublicSpace(null, "protected"); } @Test @@ -868,6 +991,10 @@ public class AuthSecurityTest { public void clearAuth() { this.clearAuthSession(); } + + public void requireSpace(HugeClient client, String graphSpace) { + this.requireGraphSpaceAccess(client, graphSpace); + } } private static class TestLoginController extends LoginController { diff --git a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/ConfigControllerTest.java b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/ConfigControllerTest.java new file mode 100644 index 000000000..a60c457d9 --- /dev/null +++ b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/ConfigControllerTest.java @@ -0,0 +1,49 @@ +/* + * + * 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; + +import java.util.Map; + +import org.junit.Assert; +import org.junit.Test; +import org.mockito.Mockito; +import org.springframework.test.util.ReflectionTestUtils; + +import org.apache.hugegraph.config.HugeConfig; +import org.apache.hugegraph.controller.ConfigController; +import org.apache.hugegraph.options.HubbleOptions; + +public class ConfigControllerTest { + + @Test + public void testBootstrapConfigDoesNotExposeBackendUrl() { + HugeConfig config = Mockito.mock(HugeConfig.class); + Mockito.when(config.get(HubbleOptions.PD_ENABLED)).thenReturn(false); + Mockito.when(config.get(HubbleOptions.AUTH_ENABLED)).thenReturn(true); + + ConfigController controller = new ConfigController(); + ReflectionTestUtils.setField(controller, "config", config); + + Map<String, Object> result = controller.getConfig(); + + Assert.assertEquals(Map.of("pd_enabled", false, + "auth_enabled", true), result); + Mockito.verify(config, Mockito.never()).get(HubbleOptions.SERVER_URL); + } +} diff --git a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/FileMappingDeletionTest.java b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/FileMappingDeletionTest.java index 1656cf852..4c5035473 100644 --- a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/FileMappingDeletionTest.java +++ b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/FileMappingDeletionTest.java @@ -25,6 +25,7 @@ import org.junit.Test; import org.mockito.Mockito; import org.apache.hugegraph.controller.load.FileMappingController; +import org.apache.hugegraph.driver.HugeClient; import org.apache.hugegraph.entity.enums.LoadStatus; import org.apache.hugegraph.entity.load.FileMapping; import org.apache.hugegraph.entity.load.LoadTask; @@ -70,7 +71,7 @@ public class FileMappingDeletionTest { private Fixture fixture(LoadStatus status) throws Exception { Fixture fixture = new Fixture(); - fixture.controller = new FileMappingController(); + fixture.controller = new TestFileMappingController(); fixture.mappingService = Mockito.mock(FileMappingService.class); fixture.jobService = Mockito.mock(JobManagerService.class); LoadTaskService taskService = Mockito.mock(LoadTaskService.class); @@ -95,9 +96,27 @@ public class FileMappingDeletionTest { private static void setField(Object target, String name, Object value) throws Exception { - Field field = target.getClass().getDeclaredField(name); - field.setAccessible(true); - field.set(target, value); + Class<?> type = target.getClass(); + while (type != null) { + try { + Field field = type.getDeclaredField(name); + field.setAccessible(true); + field.set(target, value); + return; + } catch (NoSuchFieldException ignored) { + type = type.getSuperclass(); + } + } + throw new NoSuchFieldException(name); + } + + private static class TestFileMappingController + extends FileMappingController { + + @Override + protected HugeClient requireGraphSpaceWrite(String graphSpace) { + return Mockito.mock(HugeClient.class); + } } private static final class Fixture { diff --git a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/GraphsControllerCanonicalTest.java b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/GraphsControllerCanonicalTest.java index d22567261..ca0f4f05e 100644 --- a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/GraphsControllerCanonicalTest.java +++ b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/GraphsControllerCanonicalTest.java @@ -31,8 +31,12 @@ import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.web.util.NestedServletException; import org.apache.hugegraph.controller.graphs.GraphsController; +import org.apache.hugegraph.config.HugeConfig; +import org.apache.hugegraph.controller.BaseController; import org.apache.hugegraph.driver.HugeClient; +import org.apache.hugegraph.entity.graphs.GraphCloneEntity; import org.apache.hugegraph.exception.ExternalException; +import org.apache.hugegraph.options.HubbleOptions; import org.apache.hugegraph.service.graphs.GraphsService; import org.apache.hugegraph.testutil.Assert; @@ -54,6 +58,10 @@ public class GraphsControllerCanonicalTest { GraphsController controller = new GraphsController(); this.graphsService = Mockito.mock(GraphsService.class); this.setField(controller, "graphsService", this.graphsService); + HugeConfig config = Mockito.mock(HugeConfig.class); + Mockito.when(config.get(HubbleOptions.PD_ENABLED)).thenReturn(false); + this.setField(controller, "config", config); + this.setField(BaseController.class, controller, "config", config); this.client = Mockito.mock(HugeClient.class); this.withClient = request -> { @@ -206,9 +214,44 @@ public class GraphsControllerCanonicalTest { Mockito.verify(this.graphsService).getDefault(this.client); } + @Test + public void testCloneValidatesBodyTargetGraphSpace() throws Exception { + ScopeCapturingController controller = new ScopeCapturingController(); + controller.client = this.client; + this.setField(controller, "graphsService", this.graphsService); + GraphCloneEntity clone = GraphCloneEntity.builder() + .graphSpace("target") + .name("copy") + .build(); + + controller.clone("source", "original", clone); + + Assert.assertEquals("target", controller.checkedGraphSpace); + Mockito.verify(this.graphsService).clone( + Mockito.eq(this.client), + Mockito.argThat(params -> "target".equals( + params.get("graphspace")))); + } + private void setField(Object object, String name, Object value) throws Exception { - Field field = GraphsController.class.getDeclaredField(name); + Class<?> type = object.getClass(); + while (type != null) { + try { + Field field = type.getDeclaredField(name); + field.setAccessible(true); + field.set(object, value); + return; + } catch (NoSuchFieldException ignored) { + type = type.getSuperclass(); + } + } + throw new NoSuchFieldException(name); + } + + private void setField(Class<?> type, Object object, String name, + Object value) throws Exception { + Field field = type.getDeclaredField(name); field.setAccessible(true); field.set(object, value); } @@ -218,6 +261,7 @@ public class GraphsControllerCanonicalTest { private HugeClient client; private String graphspace; private String graph; + private String checkedGraphSpace; @Override protected HugeClient authClient(String graphspace, String graph) { @@ -225,5 +269,11 @@ public class GraphsControllerCanonicalTest { this.graph = graph; return this.client; } + + @Override + protected void requireGraphSpaceAccess(HugeClient client, + String graphSpace) { + this.checkedGraphSpace = graphSpace; + } } } diff --git a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/LoaderScopeControllerTest.java b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/LoaderScopeControllerTest.java index e8f6f038f..b55def2bb 100644 --- a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/LoaderScopeControllerTest.java +++ b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/LoaderScopeControllerTest.java @@ -23,6 +23,7 @@ import java.util.Arrays; import org.apache.hugegraph.controller.load.FileMappingController; import org.apache.hugegraph.controller.load.JobManagerController; import org.apache.hugegraph.controller.load.LoadTaskController; +import org.apache.hugegraph.driver.HugeClient; import org.apache.hugegraph.entity.load.FileMapping; import org.apache.hugegraph.entity.load.JobManager; import org.apache.hugegraph.entity.load.LoadTask; @@ -39,7 +40,7 @@ public class LoaderScopeControllerTest { @Test public void testJobCreateRejectsMissingNameAsParameterError() { JobManagerService service = Mockito.mock(JobManagerService.class); - JobManagerController controller = new JobManagerController(service); + JobManagerController controller = new TestJobManagerController(service); try { controller.create("space-a", "graph-a", JobManager.builder().build()); @@ -54,7 +55,7 @@ public class LoaderScopeControllerTest { public void testJobCreateNormalizesOptionalNullRemarks() { JobManagerService service = Mockito.mock(JobManagerService.class); JobManager entity = JobManager.builder().jobName("task_1").build(); - JobManagerController controller = new JobManagerController(service); + JobManagerController controller = new TestJobManagerController(service); controller.create("space-a", "graph-a", entity); @@ -85,7 +86,7 @@ public class LoaderScopeControllerTest { .thenReturn(Collections.singletonList(mapping)); Mockito.when(taskService.taskListByJob(7)) .thenReturn(Collections.emptyList()); - FileMappingController controller = new FileMappingController(); + FileMappingController controller = new TestFileMappingController(); ReflectionTestUtils.setField(controller, "service", service); ReflectionTestUtils.setField(controller, "jobService", jobService); ReflectionTestUtils.setField(controller, "taskService", taskService); @@ -122,4 +123,26 @@ public class LoaderScopeControllerTest { Mockito.verify(service).list("space-a", "graph-a", 7, Arrays.asList(13, 14)); } + + private static class TestJobManagerController + extends JobManagerController { + + TestJobManagerController(JobManagerService service) { + super(service); + } + + @Override + protected HugeClient requireGraphSpaceWrite(String graphSpace) { + return Mockito.mock(HugeClient.class); + } + } + + private static class TestFileMappingController + extends FileMappingController { + + @Override + protected HugeClient requireGraphSpaceWrite(String graphSpace) { + return Mockito.mock(HugeClient.class); + } + } } diff --git a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/UserServiceCompatibilityTest.java b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/UserServiceCompatibilityTest.java index a3a796181..720915e25 100644 --- a/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/UserServiceCompatibilityTest.java +++ b/hugegraph-hubble/hubble-be/src/test/java/org/apache/hugegraph/unit/UserServiceCompatibilityTest.java @@ -120,12 +120,15 @@ public class UserServiceCompatibilityTest { .name("user") .nickname("display-name") .build(); + user.setSuperadmin(true); this.service.update(this.client, user); ArgumentCaptor<User> request = ArgumentCaptor.forClass(User.class); Mockito.verify(this.auth).updateUser(request.capture()); Assert.assertNull(request.getValue().nickname()); + Mockito.verify(this.auth, Mockito.never()) + .addSuperAdmin(Mockito.anyString()); } @Test @@ -291,6 +294,33 @@ public class UserServiceCompatibilityTest { .checkDefaultRole(Mockito.anyString(), Mockito.anyString()); } + @Test + public void testLegacyUserDetailKeepsMembershipWithoutPreset() { + Mockito.when(this.config.get(HubbleOptions.PD_ENABLED)).thenReturn(true); + Mockito.when(this.client.supportsDefaultRole()).thenReturn(false); + User account = user("alice"); + account.setId("user-id"); + Mockito.when(this.auth.getUser("user-id")).thenReturn(account); + Mockito.when(this.auth.listUsers()) + .thenReturn(Collections.singletonList(account)); + Mockito.when(this.graphSpace.listGraphSpace()) + .thenReturn(Collections.singletonList("SPACE")); + Mockito.when(this.auth.listSpaceAdmin("SPACE")) + .thenReturn(Collections.emptyList()); + Mockito.when(this.graphSpaceUsers.hasGraphSpaceAccess( + this.client, "SPACE", "alice")) + .thenReturn(true); + + UserEntity result = this.service.get(this.client, "user-id"); + + Assert.assertEquals(Collections.singletonList("SPACE"), + result.getResSpaces()); + Assert.assertEquals(Collections.emptyList(), + result.getGraphspacePermissions()); + Assert.assertEquals("LEGACY_CUSTOM", result.getPermissionPreset()); + Assert.assertEquals(Collections.emptyList(), result.getAdminSpaces()); + } + @Test public void testLegacyProfileUpdatePreservesPermissionAssignments() { Mockito.when(this.config.get(HubbleOptions.PD_ENABLED)).thenReturn(true); @@ -369,6 +399,33 @@ public class UserServiceCompatibilityTest { .addSuperAdmin(Mockito.anyString()); } + @Test + public void testModernCombinedPasswordAndPermissionUpdateIsRejected() { + Mockito.when(this.config.get(HubbleOptions.PD_ENABLED)).thenReturn(true); + Mockito.when(this.client.supportsDefaultRole()).thenReturn(true); + UserEntity account = UserEntity.builder() + .id("user") + .name("user") + .password("new-password") + .build(); + account.setPermissionPreset("GS_READ_ONLY"); + account.setGraphspacePermissions(Collections.singletonList( + permission("team", "GS_READ_ONLY"))); + + try { + this.service.update(this.client, account); + Assert.fail("Expected combined update to be rejected"); + } catch (ParameterizedException ignored) { + // Expected + } + + Mockito.verify(this.auth, Mockito.never()) + .updateUser(Mockito.any(User.class)); + Mockito.verify(this.graphSpaceUsers, Mockito.never()) + .applyPermissionPresets(Mockito.any(), Mockito.anyString(), + Mockito.any(), Mockito.any()); + } + @Test public void testModernUserUpdateRollsBackProfileAndSuperAdmin() { Mockito.when(this.config.get(HubbleOptions.PD_ENABLED)).thenReturn(true); @@ -411,7 +468,7 @@ public class UserServiceCompatibilityTest { } @Test - public void testLegacyUserUpdateReconcilesAdminSpaces() { + public void testLegacyUserUpdateRejectsPermissionChangesBeforeWrites() { Mockito.when(this.config.get(HubbleOptions.PD_ENABLED)).thenReturn(true); Mockito.when(this.client.supportsDefaultRole()).thenReturn(false); Mockito.when(this.graphSpace.listGraphSpace()) @@ -420,11 +477,6 @@ public class UserServiceCompatibilityTest { .thenReturn(Collections.singletonList("user")); Mockito.when(this.auth.listSpaceAdmin("NEW")) .thenReturn(Collections.emptyList()); - Mockito.when(this.client.findUserByName("user")) - .thenReturn(user("user")); - Mockito.when(this.auth.listSuperAdmin()) - .thenReturn(Collections.singletonList("user")); - Mockito.when(this.auth.getUser("user")).thenReturn(user("user")); UserEntity account = UserEntity.builder() .id("user") .name("user") @@ -432,15 +484,51 @@ public class UserServiceCompatibilityTest { Collections.singletonList("NEW")) .build(); - this.service.update(this.client, account); + ParameterizedException error = null; + try { + this.service.update(this.client, account); + } catch (ParameterizedException e) { + error = e; + } - Mockito.verify(this.auth).addSpaceAdmin("user", "NEW"); - Mockito.verify(this.auth).delSpaceAdmin("user", "OLD"); - Mockito.verify(this.auth).delSuperAdmin("user"); - Mockito.verify(this.graphSpaceUsers, Mockito.never()) - .applySpacePreset(Mockito.any(), Mockito.anyString(), - Mockito.anyString(), Mockito.anyString(), - Mockito.anyString()); + Assert.assertNotNull(error); + Assert.assertEquals("auth.permission-preset.unsupported", + error.getMessage()); + Mockito.verify(this.auth, Mockito.never()) + .addSpaceAdmin(Mockito.anyString(), Mockito.anyString()); + Mockito.verify(this.auth, Mockito.never()) + .delSpaceAdmin(Mockito.anyString(), Mockito.anyString()); + Mockito.verify(this.auth, Mockito.never()) + .updateUser(Mockito.any(User.class)); + } + + @Test + public void testLegacyUserUpdateRejectsPresetBeforeProfileWrite() { + Mockito.when(this.config.get(HubbleOptions.PD_ENABLED)).thenReturn(true); + Mockito.when(this.client.supportsDefaultRole()).thenReturn(false); + UserEntity account = UserEntity.builder() + .id("user") + .name("user") + .permissionPreset("GS_READ_ONLY") + .graphspacePermissions( + Collections.singletonList( + permission( + "SPACE", + "GS_READ_ONLY"))) + .build(); + + ParameterizedException error = null; + try { + this.service.update(this.client, account); + } catch (ParameterizedException e) { + error = e; + } + + Assert.assertNotNull(error); + Assert.assertEquals("auth.permission-preset.unsupported", + error.getMessage()); + Mockito.verify(this.auth, Mockito.never()) + .updateUser(Mockito.any(User.class)); } @Test @@ -485,7 +573,7 @@ public class UserServiceCompatibilityTest { } @Test - public void testLegacyUserCreationSkipsPermissionPresetApis() { + public void testLegacyUserCreationRejectsPermissionPreset() { Mockito.when(this.config.get(HubbleOptions.PD_ENABLED)).thenReturn(true); Mockito.when(this.client.supportsDefaultRole()).thenReturn(false); UserEntity user = userEntity("display-name"); @@ -494,9 +582,18 @@ public class UserServiceCompatibilityTest { java.util.Collections.singletonMap( "permission_preset", "GS_READ_ONLY"))); - this.service.add(this.client, user); + ParameterizedException error = null; + try { + this.service.add(this.client, user); + } catch (ParameterizedException e) { + error = e; + } - Mockito.verify(this.auth).createUser(Mockito.any(User.class)); + Assert.assertNotNull(error); + Assert.assertEquals("auth.permission-preset.unsupported", + error.getMessage()); + Mockito.verify(this.auth, Mockito.never()) + .createUser(Mockito.any(User.class)); Mockito.verify(this.graphSpaceUsers, Mockito.never()) .validatePermissionPresets(Mockito.any(), Mockito.any(), Mockito.any()); @@ -587,30 +684,46 @@ public class UserServiceCompatibilityTest { } @Test - public void testLegacyAdminGrantFailureRollsBackEarlierSpaces() { + public void testLegacyAdminGrantIsRejectedBeforeAccountCreation() { Mockito.when(this.config.get(HubbleOptions.PD_ENABLED)).thenReturn(true); Mockito.when(this.client.supportsDefaultRole()).thenReturn(false); - User created = user("user"); - created.setId("u-1"); - Mockito.when(this.auth.createUser(Mockito.any(User.class))) - .thenReturn(created); - RuntimeException failure = new RuntimeException("grant failed"); - Mockito.when(this.auth.addSpaceAdmin("user", "SECOND")) - .thenThrow(failure); UserEntity user = userEntity("display-name"); user.setAdminSpaces(Arrays.asList("FIRST", "SECOND")); - RuntimeException error = null; + ParameterizedException error = null; try { this.service.add(this.client, user); - } catch (RuntimeException e) { + } catch (ParameterizedException e) { error = e; } - Assert.assertSame(failure, error); - Mockito.verify(this.auth).delSpaceAdmin("user", "SECOND"); - Mockito.verify(this.auth).delSpaceAdmin("user", "FIRST"); - Mockito.verify(this.auth).deleteUser("u-1"); + Assert.assertNotNull(error); + Assert.assertEquals("auth.permission-preset.unsupported", + error.getMessage()); + Mockito.verify(this.auth, Mockito.never()) + .createUser(Mockito.any(User.class)); + Mockito.verify(this.auth, Mockito.never()) + .addSpaceAdmin(Mockito.anyString(), Mockito.anyString()); + } + + @Test + public void testLegacyAdminSpaceEndpointRejectsBeforeWrites() { + Mockito.when(this.config.get(HubbleOptions.PD_ENABLED)).thenReturn(true); + Mockito.when(this.client.supportsDefaultRole()).thenReturn(false); + + ParameterizedException error = null; + try { + this.service.updateAdminSpace( + this.client, "user", Collections.singletonList("SPACE")); + } catch (ParameterizedException e) { + error = e; + } + + Assert.assertNotNull(error); + Assert.assertEquals("auth.permission-preset.unsupported", + error.getMessage()); + Mockito.verify(this.auth, Mockito.never()) + .addSpaceAdmin(Mockito.anyString(), Mockito.anyString()); } @Test diff --git a/hugegraph-hubble/hubble-fe/src/api/request-error-semantics.test.js b/hugegraph-hubble/hubble-fe/src/api/request-error-semantics.test.js index febdbdc95..c95d11e38 100644 --- a/hugegraph-hubble/hubble-fe/src/api/request-error-semantics.test.js +++ b/hugegraph-hubble/hubble-fe/src/api/request-error-semantics.test.js @@ -24,6 +24,7 @@ const loadResponseHandlers = modulePath => { const modalWarning = jest.fn(); const clearLogin = jest.fn(); const isLogoutTransition = jest.fn(() => false); + const isAuthEnabled = jest.fn(() => true); const instance = { interceptors: { request: { @@ -59,6 +60,9 @@ const loadResponseHandlers = modulePath => { clearLogin, isLogoutTransition, })); + jest.doMock('../utils/config', () => ({ + isAuthEnabled, + })); const request = require(modulePath).default; return { @@ -68,6 +72,7 @@ const loadResponseHandlers = modulePath => { modalWarning, clearLogin, isLogoutTransition, + isAuthEnabled, instance, request, }; @@ -90,6 +95,7 @@ describe.each(['./request'])('%s error semantics', modulePath => { jest.dontMock('antd'); jest.dontMock('../i18n'); jest.dontMock('../utils/user'); + jest.dontMock('../utils/config'); localStorage.clear(); sessionStorage.clear(); }); @@ -239,6 +245,28 @@ describe.each(['./request'])('%s error semantics', modulePath => { expect(instance.delete).not.toHaveBeenCalled(); }); + it('keeps anonymous HTTP 401 local and shows the resource error', async () => { + const {reject, clearLogin, isAuthEnabled, messageError} + = loadResponseHandlers(modulePath); + isAuthEnabled.mockReturnValue(false); + const error = { + config: {url: '/graphspaces/protected/graphs/graph/schema'}, + response: { + status: 401, + data: { + status: 401, + message: 'GraphSpace is unavailable', + }, + }, + }; + + await expect(reject(error)).rejects.toBe(error); + expect(clearLogin).not.toHaveBeenCalled(); + expect(sessionStorage.getItem('redirect')).toBeNull(); + expect(window.location.href).toBe(''); + expect(messageError).toHaveBeenCalledWith('request.error'); + }); + it('rejects business 401 and redirects to login', async () => { const {resolve, clearLogin, instance} = loadResponseHandlers(modulePath); const response = { diff --git a/hugegraph-hubble/hubble-fe/src/api/request.js b/hugegraph-hubble/hubble-fe/src/api/request.js index 1b63ae16e..32e0aac98 100644 --- a/hugegraph-hubble/hubble-fe/src/api/request.js +++ b/hugegraph-hubble/hubble-fe/src/api/request.js @@ -25,6 +25,7 @@ import * as user from '../utils/user'; import {withLanguageHeader} from './languageHeader'; import {showThrottleWarning} from './throttleWarning'; import {AUTH_REVALIDATE_EVENT} from '../utils/authEvents'; +import {isAuthEnabled} from '../utils/config'; import {sanitizePublicError} from '../utils/publicError'; const isJsonResponse = headers => { @@ -152,9 +153,12 @@ instance.interceptors.response.use( if (isLoginRequest(response.config)) { showLoginAuthError(response); } - else { + else if (isAuthEnabled()) { redirectToLogin(); } + else { + showRequestError(response.data); + } return Promise.reject(response); } else if (response.data?.status === 429) { @@ -184,9 +188,12 @@ instance.interceptors.response.use( if (isLoginRequest(error.config)) { showLoginAuthError(error.response); } - else { + else if (isAuthEnabled()) { redirectToLogin(); } + else { + showRequestError(error.response?.data); + } return Promise.reject(error); } if (error.response?.status === 429 @@ -219,7 +226,7 @@ const request = {}; const responseData = response => { const data = response?.data; - if (data?.status === 401) { + if (data?.status === 401 && isAuthEnabled()) { redirectToLogin(); } return data; diff --git a/hugegraph-hubble/hubble-fe/src/i18n/resources/en-US/modules/pages.json b/hugegraph-hubble/hubble-fe/src/i18n/resources/en-US/modules/pages.json index 07838bafd..7547d188f 100644 --- a/hugegraph-hubble/hubble-fe/src/i18n/resources/en-US/modules/pages.json +++ b/hugegraph-hubble/hubble-fe/src/i18n/resources/en-US/modules/pages.json @@ -403,6 +403,7 @@ "save_failed": "Account was not saved", "save_retry": "Check the account fields and server connection, then retry.", "delete_retry": "The account could not be deleted. Refresh the list and retry.", + "password_permission_separate": "Save password and permission changes separately.", "presets_unavailable": "GraphSpace presets are unavailable", "presets_unavailable_help": "This deployment does not provide the required permission API. The account will be created without elevated permissions", "preset_edit_unavailable": "Permission editing is unavailable", @@ -1343,6 +1344,7 @@ "reason_refresh_failed": "Refresh failed", "reason_malformed_response": "Malformed response", "reason_unsupported_version": "Unsupported service version", + "reason_unsupported_version_help": "Upgrade HugeGraph to a version that provides this metric.", "reason_deployment_mode_unsupported": "Unsupported by the current deployment mode", "metric_labels": { "basic": "Memory & process", diff --git a/hugegraph-hubble/hubble-fe/src/i18n/resources/zh-CN/modules/pages.json b/hugegraph-hubble/hubble-fe/src/i18n/resources/zh-CN/modules/pages.json index 1efe05188..322f3e4ce 100644 --- a/hugegraph-hubble/hubble-fe/src/i18n/resources/zh-CN/modules/pages.json +++ b/hugegraph-hubble/hubble-fe/src/i18n/resources/zh-CN/modules/pages.json @@ -403,6 +403,7 @@ "save_failed": "账号未保存", "save_retry": "请检查账号字段和 Server 连接后重试。", "delete_retry": "账号删除失败，请刷新列表后重试。", + "password_permission_separate": "请分别保存密码和权限变更。", "presets_unavailable": "GraphSpace 权限预设不可用", "presets_unavailable_help": "当前部署未提供所需权限 API，账号将以无提升权限的普通账号创建", "preset_edit_unavailable": "当前无法编辑权限", @@ -1343,6 +1344,7 @@ "reason_refresh_failed": "刷新失败", "reason_malformed_response": "响应格式异常", "reason_unsupported_version": "当前服务版本不支持", + "reason_unsupported_version_help": "请升级 HugeGraph 到提供该指标的版本。", "reason_deployment_mode_unsupported": "当前部署模式不支持", "metric_labels": { "basic": "内存与进程", diff --git a/hugegraph-hubble/hubble-fe/src/pages/Account/EditLayer.js b/hugegraph-hubble/hubble-fe/src/pages/Account/EditLayer.js index df60813f8..1847ca1fd 100644 --- a/hugegraph-hubble/hubble-fe/src/pages/Account/EditLayer.js +++ b/hugegraph-hubble/hubble-fe/src/pages/Account/EditLayer.js @@ -43,6 +43,9 @@ const toProfilePayload = values => ({ user_password: values.user_password, user_description: values.user_description, }); +const sameSpaces = (left = [], right = []) => ( + [...left].sort().join('\u0000') === [...right].sort().join('\u0000') +); const loadAllGraphspaces = () => loadAllPages(api.manage.getGraphSpaceList); @@ -104,7 +107,17 @@ const EditLayer = ({ }); }, [onCancel, permissionPresetsSupported, refresh, t]); const updateUser = useCallback(values => { - const payload = permissionPresetsSupported + const initialPreset = getAccountPreset(detail) ?? PRESERVE_PERMISSIONS; + const permissionsChanged = permissionPresetsSupported + && (values.permission_preset !== initialPreset + || !sameSpaces( + values.graphspaces, + getPresetSpaces(detail) + )); + if (values.user_password && permissionsChanged) { + throw new Error(t('account.feedback.password_permission_separate')); + } + const payload = permissionsChanged && values.permission_preset !== PRESERVE_PERMISSIONS ? toPermissionPayload(values) : toProfilePayload(values); @@ -119,7 +132,7 @@ const EditLayer = ({ throw res; }); - }, [onCancel, refresh, data.id, permissionPresetsSupported, t]); + }, [onCancel, refresh, data.id, detail, permissionPresetsSupported, t]); const updateUserAuth = useCallback(values => { const payload = toPermissionPayload({ diff --git a/hugegraph-hubble/hubble-fe/src/pages/Account/account-edit-recovery.test.js b/hugegraph-hubble/hubble-fe/src/pages/Account/account-edit-recovery.test.js index 14c5e20f2..bde43ce88 100644 --- a/hugegraph-hubble/hubble-fe/src/pages/Account/account-edit-recovery.test.js +++ b/hugegraph-hubble/hubble-fe/src/pages/Account/account-edit-recovery.test.js @@ -210,6 +210,79 @@ test('preserves mixed GraphSpace permissions on profile edit', async () => { expect(payload).not.toHaveProperty('is_superadmin'); }); +test('updates a password without resubmitting unchanged permissions', async () => { + mockAuthContext = { + capabilities: ['accounts_manage', 'account_permission_presets'], + }; + api.auth.getUserInfo.mockResolvedValue({ + status: 200, + data: { + user_name: 'alice', + permission_preset: 'GS_READ_ONLY', + graphspace_permissions: [{ + graphspace: 'SPACE', + permission_preset: 'GS_READ_ONLY', + }], + }, + }); + api.auth.updateUser.mockResolvedValue({status: 200}); + + render(<EditLayer {...props} data={{id: 'alice'}} op='edit' />); + + await screen.findByDisplayValue('alice'); + const password = screen.getByPlaceholderText( + 'account.form.default_password_placeholder' + ); + fireEvent.change(password, {target: {value: 'new-password'}}); + fireEvent.click(document.querySelector( + '.ant-modal-footer .ant-btn-primary' + )); + + await waitFor(() => expect(api.auth.updateUser).toHaveBeenCalled()); + const payload = api.auth.updateUser.mock.calls[0][1]; + expect(payload.user_password).toBe('new-password'); + expect(payload).not.toHaveProperty('permission_preset'); + expect(payload).not.toHaveProperty('graphspace_permissions'); +}); + +test('rejects a combined password and permission edit before the request', async () => { + mockAuthContext = { + capabilities: ['accounts_manage', 'account_permission_presets'], + }; + api.auth.getUserInfo.mockResolvedValue({ + status: 200, + data: { + user_name: 'alice', + permission_preset: 'GS_READ_ONLY', + graphspace_permissions: [{ + graphspace: 'SPACE', + permission_preset: 'GS_READ_ONLY', + }], + }, + }); + + render(<EditLayer {...props} data={{id: 'alice'}} op='edit' />); + + await screen.findByDisplayValue('alice'); + const password = screen.getByPlaceholderText( + 'account.form.default_password_placeholder' + ); + fireEvent.change(password, {target: {value: 'new-password'}}); + fireEvent.mouseDown(screen.getAllByRole('combobox')[0]); + fireEvent.click(screen.getByText( + 'account.permission_preset.GS_READ_WRITE' + )); + fireEvent.click(document.querySelector( + '.ant-modal-footer .ant-btn-primary' + )); + + const alert = await screen.findByRole('alert'); + expect(alert).toHaveTextContent( + 'account.feedback.password_permission_separate' + ); + expect(api.auth.updateUser).not.toHaveBeenCalled(); +}); + test('ignores a second account mutation while the first submit is pending', async () => { const detailRequest = deferred(); const mutation = deferred(); diff --git a/hugegraph-hubble/hubble-fe/src/pages/Operations/NodeDetail.js b/hugegraph-hubble/hubble-fe/src/pages/Operations/NodeDetail.js index 8269d5699..4d133bc5b 100644 --- a/hugegraph-hubble/hubble-fe/src/pages/Operations/NodeDetail.js +++ b/hugegraph-hubble/hubble-fe/src/pages/Operations/NodeDetail.js @@ -16,7 +16,7 @@ * under the License. */ -import {Alert, Button, Descriptions, Progress, Skeleton, Space, Statistic} from 'antd'; +import {Alert, Button, Descriptions, Progress, Skeleton, Space, Statistic, Tooltip} from 'antd'; import {ArrowLeftOutlined} from '@ant-design/icons'; import {useCallback, useEffect, useRef, useState} from 'react'; import {useLocation, useNavigate, useParams} from 'react-router-dom'; @@ -319,6 +319,13 @@ const MetricGroup = ({group, name, values, status = {}, emptyMessage}) => { const reasonLabel = status.reason ? t(`operations.reason_${status.reason}`, { defaultValue: status.reason.replaceAll('_', ' '), }) : null; + const upgradeHelp = status.reason === 'unsupported_version' + ? t('operations.reason_unsupported_version_help') : null; + const availabilityStatus = ( + <strong className={`availability-${availability.toLowerCase()}`}> + {availabilityLabel} + </strong> + ); const statusDetails = ( <div className='operations-metric-status' @@ -336,9 +343,11 @@ const MetricGroup = ({group, name, values, status = {}, emptyMessage}) => { <header className='operations-metric-header'> <div className='operations-metric-title-row'> <h3>{name}</h3> - <strong className={`availability-${availability.toLowerCase()}`}> - {availabilityLabel} - </strong> + {upgradeHelp ? ( + <Tooltip title={upgradeHelp}> + <span tabIndex={0}>{availabilityStatus}</span> + </Tooltip> + ) : availabilityStatus} </div> {statusDetails} </header> diff --git a/hugegraph-hubble/hubble-fe/src/pages/Operations/NodeDetail.test.js b/hugegraph-hubble/hubble-fe/src/pages/Operations/NodeDetail.test.js index f3796c8eb..2d6cbc506 100644 --- a/hugegraph-hubble/hubble-fe/src/pages/Operations/NodeDetail.test.js +++ b/hugegraph-hubble/hubble-fe/src/pages/Operations/NodeDetail.test.js @@ -340,6 +340,10 @@ test('renders each metric group from its own metric status', async () => { expect(within(raft).getByText('Unsupported service version')).toBeInTheDocument(); expect(within(raft).getByText(/Unsupported by this service version/)) .toBeInTheDocument(); + fireEvent.mouseOver(within(raft).getByText('Unsupported')); + expect(await screen.findByText( + 'Upgrade HugeGraph to a version that provides this metric.' + )).toBeInTheDocument(); const backend = screen.getByRole('heading', {name: 'Backend'}).closest('section'); expect(within(backend).getByText('Available')).toBeInTheDocument();
