From 8d8526910505ae9722cc9780c5ecdd382d62efc3 Mon Sep 17 00:00:00 2001 From: Erle Czar Mantos Date: Mon, 10 Jun 2019 18:19:15 +0800 Subject: [PATCH] Fixes #1410 - API allows access to resources without Auth token Signed-off-by: Erle Czar Mantos --- .../core/impl/ModelPolicyManager.java | 61 ++++++------------- .../repository/core/impl/ModelRepository.java | 2 + .../web/api/v1/AttachmentController.java | 4 -- .../core/ModelRepositoryControllerTest.java | 2 +- 4 files changed, 22 insertions(+), 47 deletions(-) diff --git a/repository/repository-core/src/main/java/org/eclipse/vorto/repository/core/impl/ModelPolicyManager.java b/repository/repository-core/src/main/java/org/eclipse/vorto/repository/core/impl/ModelPolicyManager.java index e8d1cd1c82..e92983a46c 100644 --- a/repository/repository-core/src/main/java/org/eclipse/vorto/repository/core/impl/ModelPolicyManager.java +++ b/repository/repository-core/src/main/java/org/eclipse/vorto/repository/core/impl/ModelPolicyManager.java @@ -31,7 +31,6 @@ import org.eclipse.vorto.model.ModelId; import org.eclipse.vorto.repository.account.IUserAccountService; import org.eclipse.vorto.repository.core.IModelPolicyManager; -import org.eclipse.vorto.repository.core.ModelNotFoundException; import org.eclipse.vorto.repository.core.PolicyEntry; import org.eclipse.vorto.repository.core.PolicyEntry.Permission; import org.eclipse.vorto.repository.core.PolicyEntry.PrincipalType; @@ -58,21 +57,16 @@ public Collection getPolicyEntries(ModelId modelId) { try { ModelIdHelper modelIdHelper = new ModelIdHelper(modelId); - final Node folderNode = session.getNode(modelIdHelper.getFullPath()); - - if (!folderNode.getNodes(FILE_NODES).hasNext()) { - throw new ModelNotFoundException("Could not find model with ID " + modelId); - } - Node fileNode = folderNode.getNodes(FILE_NODES).nextNode(); - + Node nodeToGetPolicies = session.getNode(modelIdHelper.getFullPath()); + AccessControlManager acm = session.getAccessControlManager(); AccessControlList acl = null; - AccessControlPolicyIterator it = acm.getApplicablePolicies(fileNode.getPath()); + AccessControlPolicyIterator it = acm.getApplicablePolicies(nodeToGetPolicies.getPath()); if (it.hasNext()) { acl = (AccessControlList) it.nextAccessControlPolicy(); } else { - acl = (AccessControlList) acm.getPolicies(fileNode.getPath())[0]; + acl = (AccessControlList) acm.getPolicies(nodeToGetPolicies.getPath())[0]; } for (AccessControlEntry entry : acl.getAccessControlEntries()) { @@ -95,22 +89,16 @@ public void addPolicyEntry(ModelId modelId, PolicyEntry... newEntries) { try { ModelIdHelper modelIdHelper = new ModelIdHelper(modelId); - final Node folderNode = session.getNode(modelIdHelper.getFullPath()); - if (!folderNode.getNodes(FILE_NODES).hasNext()) { - logger.warn("Cannot add policy entry to model " + modelId); - session.logout(); - return null; - } - Node fileNode = folderNode.getNodes(FILE_NODES).nextNode(); - + Node nodeToAddPolicy = session.getNode(modelIdHelper.getFullPath()); + AccessControlManager acm = session.getAccessControlManager(); AccessControlList acl = null; - AccessControlPolicyIterator it = acm.getApplicablePolicies(fileNode.getPath()); + AccessControlPolicyIterator it = acm.getApplicablePolicies(nodeToAddPolicy.getPath()); if (it.hasNext()) { acl = (AccessControlList) it.nextAccessControlPolicy(); } else { - acl = (AccessControlList) acm.getPolicies(fileNode.getPath())[0]; + acl = (AccessControlList) acm.getPolicies(nodeToAddPolicy.getPath())[0]; } final AccessControlList _acl = acl; @@ -149,7 +137,7 @@ public void addPolicyEntry(ModelId modelId, PolicyEntry... newEntries) { } } - acm.setPolicy(fileNode.getPath(), _acl); + acm.setPolicy(nodeToAddPolicy.getPath(), _acl); session.save(); return null; } catch (AccessDeniedException ex) { @@ -184,20 +172,19 @@ public void removePolicyEntry(ModelId modelId, PolicyEntry entryToRemove) { try { ModelIdHelper modelIdHelper = new ModelIdHelper(modelId); - final Node folderNode = session.getNode(modelIdHelper.getFullPath()); - Node fileNode = folderNode.getNodes(FILE_NODES).nextNode(); - + Node nodeToRemovePolicy = session.getNode(modelIdHelper.getFullPath()); + AccessControlManager acm = session.getAccessControlManager(); AccessControlList acl = null; - AccessControlPolicyIterator it = acm.getApplicablePolicies(fileNode.getPath()); + AccessControlPolicyIterator it = acm.getApplicablePolicies(nodeToRemovePolicy.getPath()); if (it.hasNext()) { acl = (AccessControlList) it.nextAccessControlPolicy(); } else { - acl = (AccessControlList) acm.getPolicies(fileNode.getPath())[0]; + acl = (AccessControlList) acm.getPolicies(nodeToRemovePolicy.getPath())[0]; } - acm.removePolicy(fileNode.getPath(), acl); + acm.removePolicy(nodeToRemovePolicy.getPath(), acl); session.save(); return null; @@ -211,23 +198,13 @@ public void removePolicyEntry(ModelId modelId, PolicyEntry entryToRemove) { @Override public boolean hasPermission(final ModelId modelId, final Permission permission) { return doInSession(session -> { - try { - ModelIdHelper modelIdHelper = new ModelIdHelper(modelId); - - Node folderNode = session.getNode(modelIdHelper.getFullPath()); - - if (permission == Permission.READ) { - return folderNode.getNodes(FILE_NODES).hasNext(); - } else { - return this.getPolicyEntries(modelId).stream().filter(userFilter(session)) - .filter(p -> hasPermission(p.getPermission(), permission)).findAny().isPresent(); - } - } catch (AccessDeniedException e) { - return false; - } + return this.getPolicyEntries(modelId).stream() + .filter(userFilter(session).and(p -> hasPermission(p.getPermission(), permission))) + .findAny() + .isPresent(); }); } - + private Predicate userFilter(Session session) { return p -> { if (p.getPrincipalType() == PrincipalType.User) { diff --git a/repository/repository-core/src/main/java/org/eclipse/vorto/repository/core/impl/ModelRepository.java b/repository/repository-core/src/main/java/org/eclipse/vorto/repository/core/impl/ModelRepository.java index b3c06dc2c8..eaa9a70b64 100644 --- a/repository/repository-core/src/main/java/org/eclipse/vorto/repository/core/impl/ModelRepository.java +++ b/repository/repository-core/src/main/java/org/eclipse/vorto/repository/core/impl/ModelRepository.java @@ -730,6 +730,8 @@ public Optional getAttachmentContent(ModelId modelId, String fileNa return Optional.empty(); } catch (PathNotFoundException e) { return Optional.empty(); + } catch (AccessDeniedException e) { + throw new NotAuthorizedException(modelId); } catch (IOException | RepositoryException e) { throw new FatalModelRepositoryException("Something went wrong accessing the repository", e); } diff --git a/repository/repository-core/src/main/java/org/eclipse/vorto/repository/web/api/v1/AttachmentController.java b/repository/repository-core/src/main/java/org/eclipse/vorto/repository/web/api/v1/AttachmentController.java index 8c672ba86f..b315967633 100644 --- a/repository/repository-core/src/main/java/org/eclipse/vorto/repository/web/api/v1/AttachmentController.java +++ b/repository/repository-core/src/main/java/org/eclipse/vorto/repository/web/api/v1/AttachmentController.java @@ -125,8 +125,6 @@ private Tag[] guessTagsFromFileExtension(String fileName) { @ApiResponse(code = 404, message = "The resource could not be found")}) @RequestMapping(method = RequestMethod.GET, value = "/{modelId:.+}", produces = "application/json") - // @PreAuthorize("hasRole('ROLE_SYS_ADMIN') or - // hasPermission(T(org.eclipse.vorto.model.ModelId).fromPrettyFormat(#modelId),'model:owner')") public List getAttachments( @ApiParam( value = "The ID of the vorto model in namespace.name:version format, e.g. com.mycompany:MagneticSensor:1.0.0", @@ -156,8 +154,6 @@ public List getAttachments( value = {@ApiResponse(code = 200, message = "Successfully retrieved the attachment"), @ApiResponse(code = 404, message = "The resource could not be found")}) @RequestMapping(method = RequestMethod.GET, value = "/{modelId:.+}/files/{filename:.+}") - // @PreAuthorize("hasRole('ROLE_SYS_ADMIN') or - // hasPermission(T(org.eclipse.vorto.model.ModelId).fromPrettyFormat(#modelId),'model:owner')") public void getAttachment( @ApiParam( value = "The ID of the vorto model in namespace.name:version format, e.g. com.mycompany:MagneticSensor:1.0.0", diff --git a/repository/repository-server/src/test/java/org/eclipse/vorto/repository/web/core/ModelRepositoryControllerTest.java b/repository/repository-server/src/test/java/org/eclipse/vorto/repository/web/core/ModelRepositoryControllerTest.java index 2a3ef1347a..c705648e27 100644 --- a/repository/repository-server/src/test/java/org/eclipse/vorto/repository/web/core/ModelRepositoryControllerTest.java +++ b/repository/repository-server/src/test/java/org/eclipse/vorto/repository/web/core/ModelRepositoryControllerTest.java @@ -251,7 +251,7 @@ public void getUserPolicy() throws Exception { this.repositoryServer .perform(get("/rest" + tenant + "/models/" + testModel.prettyName + "/policy") .with(nonTenantUser)) - .andExpect(status().isNotFound()); + .andExpect(status().isUnauthorized()); this.repositoryServer.perform( get("/rest" + tenant + "/models/" + testModel.prettyName + "/policy").with(userAdmin)) .andExpect(status().isOk());