diff --git a/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/commit/CommitInfoGetRequest.java b/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/commit/CommitInfoGetRequest.java index 689e77a3a26..f63b270ad2b 100644 --- a/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/commit/CommitInfoGetRequest.java +++ b/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/commit/CommitInfoGetRequest.java @@ -15,17 +15,14 @@ */ package com.b2international.snowowl.core.commit; -import com.b2international.snowowl.core.authorization.AccessControl; import com.b2international.snowowl.core.domain.RepositoryContext; -import com.b2international.snowowl.core.identity.Permission; import com.b2international.snowowl.core.request.GetResourceRequest; /** * @since 5.7 */ final class CommitInfoGetRequest - extends GetResourceRequest - implements AccessControl { + extends GetResourceRequest { private static final long serialVersionUID = 1L; @@ -37,10 +34,5 @@ final class CommitInfoGetRequest protected CommitInfoSearchRequestBuilder createSearchRequestBuilder() { return new CommitInfoSearchRequestBuilder(); } - - @Override - public String getOperation() { - return Permission.OPERATION_BROWSE; - } - + } diff --git a/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/commit/CommitInfoSearchRequest.java b/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/commit/CommitInfoSearchRequest.java index ee7fa14c573..b428b47ef11 100644 --- a/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/commit/CommitInfoSearchRequest.java +++ b/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/commit/CommitInfoSearchRequest.java @@ -15,17 +15,31 @@ */ package com.b2international.snowowl.core.commit; +import static com.b2international.index.query.Expressions.regexp; import static com.b2international.index.revision.Commit.Expressions.*; +import static com.b2international.index.revision.Commit.Fields.BRANCH; +import static com.b2international.index.revision.RevisionBranch.DEFAULT_MAXIMUM_BRANCH_NAME_LENGTH; +import static com.b2international.snowowl.core.identity.Permission.isWildCardResource; import java.util.Collection; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; import com.b2international.index.Hits; import com.b2international.index.query.Expression; import com.b2international.index.query.Expressions; import com.b2international.index.query.Expressions.ExpressionBuilder; import com.b2international.index.revision.Commit; +import com.b2international.snowowl.core.TerminologyResource; import com.b2international.snowowl.core.domain.RepositoryContext; +import com.b2international.snowowl.core.identity.Permission; +import com.b2international.snowowl.core.identity.User; +import com.b2international.snowowl.core.internal.ResourceDocument; +import com.b2international.snowowl.core.request.ResourceRequests; import com.b2international.snowowl.core.request.SearchIndexResourceRequest; +import com.google.common.collect.Sets; +import com.google.common.collect.Sets.SetView; /** * @since 5.2 @@ -58,8 +72,8 @@ protected Class getDocumentType() { protected Expression prepareQuery(RepositoryContext context) { ExpressionBuilder queryBuilder = Expressions.builder(); addIdFilter(queryBuilder, Commit.Expressions::ids); - // TODO add a security filter to return commits from resources that can be accessed by the current user - addBranchClause(queryBuilder); + addSecurityFilter(queryBuilder, context); + addBranchClause(queryBuilder, context); addBranchPrefixClause(queryBuilder); addUserIdClause(queryBuilder); addCommentClause(queryBuilder); @@ -88,13 +102,60 @@ protected CommitInfos createEmptyResult(int limit) { return new CommitInfos(limit, 0); } - private void addBranchClause(final ExpressionBuilder builder) { + private void addSecurityFilter(final ExpressionBuilder builder, RepositoryContext context) { + + final User user = context.service(User.class); + if (user.isAdministrator() || user.hasPermission(Permission.requireAll(Permission.OPERATION_BROWSE, Permission.ALL))) { + return; + } + + final List readPermissions = user.getPermissions().stream() + .filter(p -> Permission.ALL.equals(p.getOperation()) || Permission.OPERATION_BROWSE.equals(p.getOperation())) + .collect(Collectors.toList()); + + final Set exactResourceIds = readPermissions.stream() + .flatMap(p -> p.getResources().stream()) + .filter(resource -> !resource.endsWith("*")) + .collect(Collectors.toSet()); + + final Set resourceIdPrefixes = readPermissions.stream() + .flatMap(p -> p.getResources().stream()) + .filter(resource -> isWildCardResource(resource)) + .map(resource -> resource.substring(0, resource.length() - 1)) + .collect(Collectors.toSet()); + + SetView resourceIds = Sets.union(exactResourceIds, resourceIdPrefixes); + + ExpressionBuilder branchFilter = Expressions.builder(); + ResourceRequests.prepareSearch() + .filterByIds(resourceIds) + .setLimit(resourceIds.size()) + .setFields(ResourceDocument.Fields.ID, + ResourceDocument.Fields.BRANCH_PATH, + ResourceDocument.Fields.RESOURCE_TYPE) + .buildAsync() + .getRequest() + .execute(context) + .stream() + .filter(TerminologyResource.class::isInstance) + .map(TerminologyResource.class::cast) + .forEach(r -> { + if (resourceIdPrefixes.contains(r.getId())) { + final String branchPattern = String.format("%s(/[a-zA-Z0-9.~_\\-]{1,%d})?", r.getBranchPath(), DEFAULT_MAXIMUM_BRANCH_NAME_LENGTH); + branchFilter.should(regexp(BRANCH, branchPattern)); + } + }); + + builder.filter(branchFilter.build()); + } + + private void addBranchClause(final ExpressionBuilder builder, RepositoryContext context) { if (containsKey(OptionKey.BRANCH)) { final Collection branchPaths = getCollection(OptionKey.BRANCH, String.class); builder.filter(branches(branchPaths)); } } - + private void addUserIdClause(final ExpressionBuilder builder) { if (containsKey(OptionKey.AUTHOR)) { final String userId = getString(OptionKey.AUTHOR); diff --git a/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/identity/Permission.java b/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/identity/Permission.java index abfa8775575..69184033210 100644 --- a/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/identity/Permission.java +++ b/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/identity/Permission.java @@ -134,4 +134,8 @@ static Permission valueOf(@JsonProperty("permission") final String permission) { } } + static boolean isWildCardResource(String resource) { + return resource.endsWith("*"); + } + } diff --git a/snomed/com.b2international.snowowl.snomed.core.rest.tests/src/com/b2international/snowowl/snomed/core/io/commit/CommitInfoRequestTest.java b/snomed/com.b2international.snowowl.snomed.core.rest.tests/src/com/b2international/snowowl/snomed/core/io/commit/CommitInfoRequestTest.java index 66921c11447..7bc062fdef4 100644 --- a/snomed/com.b2international.snowowl.snomed.core.rest.tests/src/com/b2international/snowowl/snomed/core/io/commit/CommitInfoRequestTest.java +++ b/snomed/com.b2international.snowowl.snomed.core.rest.tests/src/com/b2international/snowowl/snomed/core/io/commit/CommitInfoRequestTest.java @@ -18,20 +18,36 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import java.util.List; import java.util.UUID; +import org.elasticsearch.core.Map; import org.junit.Before; import org.junit.Test; import com.b2international.commons.exceptions.NotFoundException; +import com.b2international.snowowl.core.ResourceURI; +import com.b2international.snowowl.core.authorization.AuthorizedEventBus; +import com.b2international.snowowl.core.authorization.AuthorizedRequest; import com.b2international.snowowl.core.branch.Branch; +import com.b2international.snowowl.core.codesystem.CodeSystem; import com.b2international.snowowl.core.codesystem.CodeSystemRequests; import com.b2international.snowowl.core.commit.CommitInfo; import com.b2international.snowowl.core.commit.CommitInfos; +import com.b2international.snowowl.core.context.ResourceRepositoryRequestBuilder; +import com.b2international.snowowl.core.domain.RepositoryContext; +import com.b2international.snowowl.core.events.Request; +import com.b2international.snowowl.core.identity.JWTGenerator; +import com.b2international.snowowl.core.identity.Permission; +import com.b2international.snowowl.core.identity.Role; +import com.b2international.snowowl.core.identity.User; import com.b2international.snowowl.core.repository.RepositoryRequests; import com.b2international.snowowl.eventbus.IEventBus; +import com.b2international.snowowl.snomed.common.SnomedConstants.Concepts; import com.b2international.snowowl.snomed.common.SnomedTerminologyComponentConstants; +import com.b2international.snowowl.snomed.datastore.request.SnomedRequests; import com.b2international.snowowl.test.commons.Services; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; /** @@ -61,31 +77,144 @@ public void getNonExistentCommitInfo() { } @Test - public void getCommitInfo() { + public void searchCommitInfoByBranch() { final String oid = UUID.randomUUID().toString(); - final String shortName = UUID.randomUUID().toString(); - final String comment = "Code system for commit info 1"; + final String shortName = "Resource6"; + final String comment = "Code system for commit info 6"; + final String branchName = "Test6"; + final String term = "Test Description 6"; + + createCodeSystem(shortName, oid, comment); - createCodeSystem(oid, shortName, comment); + final String branchPath = createBranch(String.format("%s/%s", BRANCH, shortName), branchName); + createDescription(ResourceURI.branch(CodeSystem.RESOURCE_TYPE, shortName, branchName), term, comment); + + //Search as admin + assertEquals(1, RepositoryRequests + .commitInfos() + .prepareSearchCommitInfo() + .filterByBranch(branchPath) + .build(REPOSITORY_ID) + .execute(bus) + .getSync().getTotal()); - final CommitInfos commitInfos = RepositoryRequests + final Permission userPermission = Permission.requireAll(Permission.OPERATION_BROWSE, String.format("%s*", shortName)); + final List roles = List.of(new Role("Editor", List.of(userPermission))); + final String userName = "User6"; + final User user = new User(userName, roles); + final IEventBus authorizedBus = new AuthorizedEventBus(bus, + ImmutableMap.of(AuthorizedRequest.AUTHORIZATION_HEADER, Services.service(JWTGenerator.class).generate(user)) + ); + + //Search as user with limited permissions + assertEquals(1, RepositoryRequests .commitInfos() .prepareSearchCommitInfo() - .filterByComment(comment) + .filterByBranch(branchPath) .build(REPOSITORY_ID) + .execute(authorizedBus) + .getSync() + .getTotal()); + } + + private void createDescription(final ResourceURI uri, final String term, final String commitComment) { + SnomedRequests.prepareNewDescription() + .setAcceptability(Map.of()) + .setActive(true) + .setCaseSignificanceId(Concepts.ENTIRE_TERM_CASE_INSENSITIVE) + .setConceptId(Concepts.ROOT_CONCEPT) + .setIdFromNamespace("") + .setLanguageCode("en-US") + .setModuleId(Concepts.MODULE_SCT_CORE) + .setTerm(term) + .setTypeId(Concepts.SYNONYM) + .build(uri, USER_ID, commitComment) + .execute(bus) + .getSync(); + } + + private String createBranch(final String parent, final String name) { + return RepositoryRequests.branching() + .prepareCreate() + .setParent(parent) + .setName(name) + .build(SnomedTerminologyComponentConstants.TOOLING_ID) .execute(bus) .getSync(); + } + + @Test + public void searchCommitOnSubBranch() { + //Search with no branch filter, to test security filter for user with limited resources + final String oid = UUID.randomUUID().toString(); + final String shortName = "Resource7"; + final String comment = "Code system for commit info 7"; + final String branchName = "Test7"; + final String commitComment = "Create Description 7"; + final String term = "Test Description 7"; - assertEquals(commitInfos.getTotal(), 1); + //Commit on resource branch + createCodeSystem(shortName, oid, comment); + createDescription(ResourceURI.of(CodeSystem.RESOURCE_TYPE, shortName), term, commitComment); + + //Commit on version branch + final String branchPath = createBranch(String.format("%s/%s", BRANCH, shortName), branchName); + createDescription(ResourceURI.branch(CodeSystem.RESOURCE_TYPE, shortName, branchName), term, commitComment); - final String id = Iterables.getOnlyElement(commitInfos).getId(); + //Commit on deeper branch + final String newBranchName = String.format("%s/%s", branchName, branchName); + createBranch(branchPath, branchName); + createDescription(ResourceURI.branch(CodeSystem.RESOURCE_TYPE, shortName, newBranchName), term, commitComment); + + final Permission userPermission = Permission.requireAll(Permission.OPERATION_BROWSE, String.format("%s*", shortName)); + final List roles = List.of(new Role("Editor", List.of(userPermission))); + final String userName = "User7"; + final User user = new User(userName, roles); + final IEventBus authorizedBus = new AuthorizedEventBus(bus, + ImmutableMap.of(AuthorizedRequest.AUTHORIZATION_HEADER, Services.service(JWTGenerator.class).generate(user)) + ); - final CommitInfo commitInfo = RepositoryRequests + //Search as user with permission only to access the resource and one sub branch + assertEquals(2, RepositoryRequests .commitInfos() - .prepareGetCommitInfo(id) + .prepareSearchCommitInfo() + .filterByComment(commitComment) + .build(REPOSITORY_ID) + .execute(authorizedBus) + .getSync() + .getTotal()); + + //Search as admin user with permission to access all + assertEquals(3, RepositoryRequests + .commitInfos() + .prepareSearchCommitInfo() + .filterByComment(commitComment) .build(REPOSITORY_ID) .execute(bus) - .getSync(); + .getSync() + .getTotal()); + } + + @Test + public void getCommitInfo() { + final String oid = UUID.randomUUID().toString(); + final String shortName = UUID.randomUUID().toString(); + final String comment = "Code system for commit info 1"; + + createCodeSystem(oid, shortName, comment); + final String id = getCommitInfoByComment(comment).getId(); + + Request req = RepositoryRequests + .commitInfos() + .prepareGetCommitInfo(id) + .build(); + + CommitInfo commitInfo = new ResourceRepositoryRequestBuilder() { + @Override + public Request build() { + return req; + } + }.buildAsync().execute(bus).getSync(); assertEquals(id, commitInfo.getId()); assertEquals(comment, commitInfo.getComment()); @@ -114,55 +243,46 @@ public void searchCommitInfoByUserId() { createCodeSystem(oid, shortName, comment, userId); - final CommitInfos commitInfos = RepositoryRequests + Request req = RepositoryRequests .commitInfos() .prepareSearchCommitInfo() .filterByAuthor(userId) - .build(REPOSITORY_ID) - .execute(bus) - .getSync(); + .build(); + + final CommitInfos commitInfos = new ResourceRepositoryRequestBuilder() { + @Override + public Request build() { + return req; + } + }.buildAsync().execute(bus).getSync(); assertEquals(commitInfos.getTotal(), 1); final CommitInfo commitInfo = Iterables.getOnlyElement(commitInfos); assertEquals(userId, commitInfo.getAuthor()); } - - @Test - public void searchCommitInfoByBranch() { - final String oid = UUID.randomUUID().toString(); - final String shortName = UUID.randomUUID().toString(); - final String comment = "Code system for commit info 4"; - - createCodeSystem(oid, shortName, comment); - final CommitInfos commitInfos = RepositoryRequests - .commitInfos() - .prepareSearchCommitInfo() - .filterByBranch(BRANCH) - .build(REPOSITORY_ID) - .execute(bus) - .getSync(); - - assertTrue(commitInfos.getTotal() >= 1); - } - @Test public void searchCommitInfoByTimestamp() { final String oid = UUID.randomUUID().toString(); final String shortName = UUID.randomUUID().toString(); final String comment = "Code system for commit info 5"; - createCodeSystem(oid, shortName, comment); + createCodeSystem(shortName, oid, comment); final CommitInfo commitInfo = getCommitInfoByComment(comment); - final CommitInfos commitInfos = RepositoryRequests + Request req = RepositoryRequests .commitInfos() .prepareSearchCommitInfo() .filterByTimestamp(commitInfo.getTimestamp()) - .build(REPOSITORY_ID) - .execute(bus) - .getSync(); + .build(); + + final CommitInfos commitInfos = new ResourceRepositoryRequestBuilder() { + @Override + public Request build() { + return req; + } + }.buildAsync().execute(bus).getSync(); assertTrue(commitInfos.getTotal() == 1); assertEquals(commitInfo.getTimestamp(), Iterables.getOnlyElement(commitInfos.getItems()).getTimestamp()); @@ -176,10 +296,9 @@ private void createCodeSystem(final String shortName, final String oid, final St CodeSystemRequests.prepareNewCodeSystem() .setId(shortName) .setOid(oid) - .setUrl("www.ihtsdo.org") + .setUrl(String.format("http://snomed.info/sct/%s", shortName)) .setTitle(String.format("%s - %s", shortName, oid)) .setLanguage("en") - .setBranchPath(BRANCH) .setDescription("citation") .setToolingId(SnomedTerminologyComponentConstants.TOOLING_ID) .build(userId, comment) @@ -188,14 +307,24 @@ private void createCodeSystem(final String shortName, final String oid, final St } private CommitInfo getCommitInfoByComment(final String comment) { - final CommitInfos commitInfos = RepositoryRequests + Request req = RepositoryRequests .commitInfos() .prepareSearchCommitInfo() .filterByComment(comment) - .build(REPOSITORY_ID) - .execute(bus) - .getSync(); + .build(); + final CommitInfos commitInfos = new ResourceRepositoryRequestBuilder() { + + @Override + public Request build() { + return req; + } + + } + .buildAsync() + .execute(bus) + .getSync(); + assertEquals(commitInfos.getTotal(), 1); return Iterables.getOnlyElement(commitInfos); diff --git a/snomed/com.b2international.snowowl.snomed.core.rest.tests/src/com/b2international/snowowl/snomed/core/rest/AllSnomedApiTests.java b/snomed/com.b2international.snowowl.snomed.core.rest.tests/src/com/b2international/snowowl/snomed/core/rest/AllSnomedApiTests.java index c38d9b262cd..78695c40c76 100644 --- a/snomed/com.b2international.snowowl.snomed.core.rest.tests/src/com/b2international/snowowl/snomed/core/rest/AllSnomedApiTests.java +++ b/snomed/com.b2international.snowowl.snomed.core.rest.tests/src/com/b2international/snowowl/snomed/core/rest/AllSnomedApiTests.java @@ -26,6 +26,7 @@ import com.b2international.snowowl.snomed.core.branch.SnomedBranchRequestTest; import com.b2international.snowowl.snomed.core.domain.Rf2ReleaseType; import com.b2international.snowowl.snomed.core.io.SnomedRefSetDSVExportTest; +import com.b2international.snowowl.snomed.core.io.commit.CommitInfoRequestTest; import com.b2international.snowowl.snomed.core.issue.EclSerializerTest; import com.b2international.snowowl.snomed.core.issue.IssueSO2503RemoteJobDynamicMappingFix; import com.b2international.snowowl.snomed.core.request.ConceptMapSearchMappingRequestSnomedMapTypeReferenceSetTest; @@ -120,6 +121,7 @@ SnomedReferenceSetDeletionPerformanceTest.class, SnomedConceptCreatePerformanceTest.class, SnomedMergePerformanceTest.class, + CommitInfoRequestTest.class }) public class AllSnomedApiTests { diff --git a/snomed/com.b2international.snowowl.snomed.core.rest.tests/src/configuration/snowowl.yml b/snomed/com.b2international.snowowl.snomed.core.rest.tests/src/configuration/snowowl.yml index decf0b2d152..873b3d71ae5 100644 --- a/snomed/com.b2international.snowowl.snomed.core.rest.tests/src/configuration/snowowl.yml +++ b/snomed/com.b2international.snowowl.snomed.core.rest.tests/src/configuration/snowowl.yml @@ -3,6 +3,8 @@ # identity: + jws: HS512 + secret: secret providers: - file: name: users