From bc9c1b2ee7aa7241b646d622581c508c9aa99679 Mon Sep 17 00:00:00 2001 From: Assad Riaz Date: Tue, 8 Oct 2024 11:50:59 +0200 Subject: [PATCH] Refactoring add test --- .../tiamat/auth/AuthorizationService.java | 8 ++-- .../auth/DefaultAuthorizationService.java | 28 ++++++----- .../config/AuthorizationServiceConfig.java | 8 ++-- .../fetchers/EntityPermissionsFetcher.java | 8 ++-- .../auth/DefaultAuthorizationServiceTest.java | 4 +- .../StopPlaceAuthorizationServiceTest.java | 2 +- .../auth/TiamatAuthorizationServiceTest.java | 47 +++++++++++++++++-- 7 files changed, 75 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/rutebanken/tiamat/auth/AuthorizationService.java b/src/main/java/org/rutebanken/tiamat/auth/AuthorizationService.java index c77d87d44..524a4ae35 100644 --- a/src/main/java/org/rutebanken/tiamat/auth/AuthorizationService.java +++ b/src/main/java/org/rutebanken/tiamat/auth/AuthorizationService.java @@ -58,13 +58,13 @@ public interface AuthorizationService { */ boolean canEditEntity(RoleAssignment roleAssignment, T entity); - Set getAllowedStopPlaceTypes(); + Set getAllowedStopPlaceTypes(Object entity); - Set getBannedStopPlaceTypes(); + Set getBannedStopPlaceTypes(Object entity); - Set getAllowedSubmodes(); + Set getAllowedSubmodes(Object entity); - Set getBannedSubmodes(); + Set getBannedSubmodes(Object entity); diff --git a/src/main/java/org/rutebanken/tiamat/auth/DefaultAuthorizationService.java b/src/main/java/org/rutebanken/tiamat/auth/DefaultAuthorizationService.java index 1eaf8bda5..7a95eb9c5 100644 --- a/src/main/java/org/rutebanken/tiamat/auth/DefaultAuthorizationService.java +++ b/src/main/java/org/rutebanken/tiamat/auth/DefaultAuthorizationService.java @@ -5,6 +5,7 @@ import org.rutebanken.helper.organisation.DataScopedAuthorizationService; import org.rutebanken.helper.organisation.RoleAssignment; import org.rutebanken.helper.organisation.RoleAssignmentExtractor; +import org.rutebanken.tiamat.auth.check.TopographicPlaceChecker; import org.rutebanken.tiamat.model.EntityStructure; import org.springframework.security.access.AccessDeniedException; @@ -22,11 +23,15 @@ public class DefaultAuthorizationService implements AuthorizationService { private final RoleAssignmentExtractor roleAssignmentExtractor; private static final String STOP_PLACE_TYPE = "StopPlaceType"; private static final String SUBMODE = "Submode"; + private final TopographicPlaceChecker topographicPlaceChecker; - public DefaultAuthorizationService(DataScopedAuthorizationService dataScopedAuthorizationService, RoleAssignmentExtractor roleAssignmentExtractor) { + public DefaultAuthorizationService(DataScopedAuthorizationService dataScopedAuthorizationService, + RoleAssignmentExtractor roleAssignmentExtractor, + TopographicPlaceChecker topographicPlaceChecker) { this.dataScopedAuthorizationService = dataScopedAuthorizationService; this.roleAssignmentExtractor = roleAssignmentExtractor; - } + this.topographicPlaceChecker = topographicPlaceChecker; + } @Override public void verifyCanEditAllEntities() { @@ -83,29 +88,30 @@ public boolean canEditEntity(EntityStructure entity) { } @Override - public Set getAllowedStopPlaceTypes(){ - return getStopTypesOrSubmode(STOP_PLACE_TYPE, true); + public Set getAllowedStopPlaceTypes(Object entity){ + return getStopTypesOrSubmode(STOP_PLACE_TYPE, true, entity); } @Override - public Set getBannedStopPlaceTypes() { - return getStopTypesOrSubmode(STOP_PLACE_TYPE, false); + public Set getBannedStopPlaceTypes(Object entity) { + return getStopTypesOrSubmode(STOP_PLACE_TYPE, false, entity); } @Override - public Set getAllowedSubmodes() { - return getStopTypesOrSubmode(SUBMODE, true); + public Set getAllowedSubmodes(Object entity) { + return getStopTypesOrSubmode(SUBMODE, true, entity); } @Override - public Set getBannedSubmodes() { - return getStopTypesOrSubmode(SUBMODE, false); + public Set getBannedSubmodes(Object entity) { + return getStopTypesOrSubmode(SUBMODE, false, entity); } - private Set getStopTypesOrSubmode(String type, boolean isAllowed) { + private Set getStopTypesOrSubmode(String type, boolean isAllowed, Object entity) { return roleAssignmentExtractor.getRoleAssignmentsForUser().stream() .filter(roleAssignment -> roleAssignment.getEntityClassifications() != null) + .filter(roleAssignment -> topographicPlaceChecker.entityMatchesAdministrativeZone(roleAssignment, entity)) .filter(roleAssignment -> roleAssignment.getEntityClassifications().get(type) != null) .map(roleAssignment -> roleAssignment.getEntityClassifications().get(type)) .flatMap(List::stream) diff --git a/src/main/java/org/rutebanken/tiamat/config/AuthorizationServiceConfig.java b/src/main/java/org/rutebanken/tiamat/config/AuthorizationServiceConfig.java index e0a68fda2..24187dac7 100644 --- a/src/main/java/org/rutebanken/tiamat/config/AuthorizationServiceConfig.java +++ b/src/main/java/org/rutebanken/tiamat/config/AuthorizationServiceConfig.java @@ -18,11 +18,11 @@ import org.rutebanken.helper.organisation.DataScopedAuthorizationService; import org.rutebanken.helper.organisation.ReflectionAuthorizationService; import org.rutebanken.helper.organisation.RoleAssignmentExtractor; +import org.rutebanken.tiamat.auth.AuthorizationService; +import org.rutebanken.tiamat.auth.DefaultAuthorizationService; import org.rutebanken.tiamat.auth.TiamatEntityResolver; import org.rutebanken.tiamat.auth.check.TiamatOriganisationChecker; import org.rutebanken.tiamat.auth.check.TopographicPlaceChecker; -import org.rutebanken.tiamat.auth.AuthorizationService; -import org.rutebanken.tiamat.auth.DefaultAuthorizationService; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -39,8 +39,8 @@ public class AuthorizationServiceConfig { @Bean - public AuthorizationService authorizationService(DataScopedAuthorizationService dataScopedAuthorizationService, RoleAssignmentExtractor roleAssignmentExtractor) { - return new DefaultAuthorizationService(dataScopedAuthorizationService, roleAssignmentExtractor); + public AuthorizationService authorizationService(DataScopedAuthorizationService dataScopedAuthorizationService, RoleAssignmentExtractor roleAssignmentExtractor, TopographicPlaceChecker topographicPlaceChecker) { + return new DefaultAuthorizationService(dataScopedAuthorizationService, roleAssignmentExtractor, topographicPlaceChecker); } @Bean diff --git a/src/main/java/org/rutebanken/tiamat/rest/graphql/fetchers/EntityPermissionsFetcher.java b/src/main/java/org/rutebanken/tiamat/rest/graphql/fetchers/EntityPermissionsFetcher.java index a05a08c86..c7cde3ae9 100644 --- a/src/main/java/org/rutebanken/tiamat/rest/graphql/fetchers/EntityPermissionsFetcher.java +++ b/src/main/java/org/rutebanken/tiamat/rest/graphql/fetchers/EntityPermissionsFetcher.java @@ -41,10 +41,10 @@ public Object get(DataFetchingEnvironment environment) throws Exception { final boolean canEditEntities = authorizationService.canEditEntity(entityInVersionStructure); final boolean canDeleteEntity = authorizationService.canDeleteEntity(entityInVersionStructure); - final Set allowedStopPlaceTypes = authorizationService.getAllowedStopPlaceTypes(); - final Set bannedStopPlaceTypes = authorizationService.getBannedStopPlaceTypes(); - final Set allowedSubmode = authorizationService.getAllowedSubmodes(); - final Set bannedSubmode = authorizationService.getBannedSubmodes(); + final Set allowedStopPlaceTypes = authorizationService.getAllowedStopPlaceTypes(entityInVersionStructure); + final Set bannedStopPlaceTypes = authorizationService.getBannedStopPlaceTypes(entityInVersionStructure); + final Set allowedSubmode = authorizationService.getAllowedSubmodes(entityInVersionStructure); + final Set bannedSubmode = authorizationService.getBannedSubmodes(entityInVersionStructure); return new EntityPermissions(canEditEntities, canDeleteEntity, allowedStopPlaceTypes, bannedStopPlaceTypes, allowedSubmode, bannedSubmode); diff --git a/src/test/java/org/rutebanken/tiamat/auth/DefaultAuthorizationServiceTest.java b/src/test/java/org/rutebanken/tiamat/auth/DefaultAuthorizationServiceTest.java index 248d48d9e..62e3182e5 100644 --- a/src/test/java/org/rutebanken/tiamat/auth/DefaultAuthorizationServiceTest.java +++ b/src/test/java/org/rutebanken/tiamat/auth/DefaultAuthorizationServiceTest.java @@ -12,14 +12,14 @@ class DefaultAuthorizationServiceTest { @Test void verifyCanEditAllEntities() { List roleAssignments = RoleAssignmentListBuilder.builder().withAccessAllAreas().build(); - DefaultAuthorizationService defaultAuthorizationService = new DefaultAuthorizationService(null, null); + DefaultAuthorizationService defaultAuthorizationService = new DefaultAuthorizationService(null, null, null); Assertions.assertDoesNotThrow(() -> defaultAuthorizationService.verifyCanEditAllEntities(roleAssignments)); } @Test void verifyCanEditAllEntitiesMissingRoleAssignment() { List roleAssignments = RoleAssignmentListBuilder.builder().build(); - DefaultAuthorizationService defaultAuthorizationService = new DefaultAuthorizationService(null, null); + DefaultAuthorizationService defaultAuthorizationService = new DefaultAuthorizationService(null, null, null); Assertions.assertThrows(AccessDeniedException.class, () -> defaultAuthorizationService.verifyCanEditAllEntities(roleAssignments)); } } \ No newline at end of file diff --git a/src/test/java/org/rutebanken/tiamat/auth/StopPlaceAuthorizationServiceTest.java b/src/test/java/org/rutebanken/tiamat/auth/StopPlaceAuthorizationServiceTest.java index 67dbe1f20..9538f9e07 100644 --- a/src/test/java/org/rutebanken/tiamat/auth/StopPlaceAuthorizationServiceTest.java +++ b/src/test/java/org/rutebanken/tiamat/auth/StopPlaceAuthorizationServiceTest.java @@ -122,7 +122,7 @@ public void StopPlaceAuthorizationServiceTest() { tiamatOriganisationChecker, topographicPlaceChecker, tiamatEntityResolver); - this.authorizationService = authorizationServiceConfig.authorizationService(dataScopedAuthorizationService, roleAssignmentExtractor); + this.authorizationService = authorizationServiceConfig.authorizationService(dataScopedAuthorizationService, roleAssignmentExtractor,topographicPlaceChecker); diff --git a/src/test/java/org/rutebanken/tiamat/auth/TiamatAuthorizationServiceTest.java b/src/test/java/org/rutebanken/tiamat/auth/TiamatAuthorizationServiceTest.java index 662e51366..7edb2876e 100644 --- a/src/test/java/org/rutebanken/tiamat/auth/TiamatAuthorizationServiceTest.java +++ b/src/test/java/org/rutebanken/tiamat/auth/TiamatAuthorizationServiceTest.java @@ -16,12 +16,19 @@ package org.rutebanken.tiamat.auth; import org.junit.Test; +import org.locationtech.jts.geom.Coordinate; +import org.locationtech.jts.geom.Geometry; +import org.locationtech.jts.geom.LinearRing; +import org.locationtech.jts.geom.Point; +import org.locationtech.jts.geom.Polygon; +import org.locationtech.jts.geom.impl.CoordinateArraySequence; import org.rutebanken.helper.organisation.RoleAssignment; import org.rutebanken.tiamat.TiamatIntegrationTest; import org.rutebanken.tiamat.model.BusSubmodeEnumeration; import org.rutebanken.tiamat.model.Quay; import org.rutebanken.tiamat.model.StopPlace; import org.rutebanken.tiamat.model.StopTypeEnumeration; +import org.rutebanken.tiamat.model.TopographicPlace; import org.rutebanken.tiamat.model.WaterSubmodeEnumeration; import org.rutebanken.tiamat.repository.StopPlaceRepository; import org.springframework.beans.factory.annotation.Autowired; @@ -151,9 +158,13 @@ public void authorizedWithSubmodeAndType() { @Test public void authorizedGetAllowedStopPlaceTypesTest() { - roleAssignmentsForRailAndRailReplacementMocked(ROLE_EDIT_STOPS); + final List roleAssignments = roleAssignmentsForRailAndRailReplacementMocked(ROLE_EDIT_STOPS); + + StopPlace stopPlace = new StopPlace(); + stopPlace.setStopPlaceType(StopTypeEnumeration.BUS_STATION); + stopPlace.setBusSubmode(BusSubmodeEnumeration.REGIONAL_BUS); - final Set allowedStopPlaceTypes = authorizationService.getAllowedStopPlaceTypes(); + final Set allowedStopPlaceTypes = authorizationService.getAllowedStopPlaceTypes(stopPlace); assertThat("Should contain allowed StopPlaceType", allowedStopPlaceTypes.contains("railStation"), is(true)); } @@ -166,6 +177,7 @@ public void authorizedGetBannedStopPlaceTypesTest() { RoleAssignment roleAssignment = RoleAssignment.builder() .withRole(ROLE_EDIT_STOPS) .withOrganisation("OST") + .withAdministrativeZone("KVE:TopographicalPlace:01") .withEntityClassification(ENTITY_TYPE, "StopPlace") .withEntityClassification("StopPlaceType", "!airport") .withEntityClassification("Submode", "!railReplacementBus") @@ -173,8 +185,35 @@ public void authorizedGetBannedStopPlaceTypesTest() { mockedRoleAssignmentExtractor.setNextReturnedRoleAssignment(roleAssignment); - final Set bannedStopPlaceTypes = authorizationService.getBannedStopPlaceTypes(); - assertThat("Should contain banned StopPlaceType", bannedStopPlaceTypes.contains("airport"), is(true)); + Point point = geometryFactory.createPoint(new Coordinate(9.84, 59.26)); + Point point2 = geometryFactory.createPoint(new Coordinate(0, 0)); + + TopographicPlace municipality = new TopographicPlace(); + municipality.setNetexId("KVE:TopographicalPlace:01"); + municipality.setVersion(1); + municipality.setPolygon(createPolygon(point)); + topographicPlaceRepository.saveAndFlush(municipality); + + + + StopPlace stopPlace = new StopPlace(); + stopPlace.setStopPlaceType(StopTypeEnumeration.BUS_STATION); + stopPlace.setBusSubmode(BusSubmodeEnumeration.REGIONAL_BUS); + stopPlace.setTopographicPlace(municipality); + stopPlace.setCentroid(point2); + stopPlaceRepository.saveAndFlush(stopPlace); + + final Set bannedStopPlaceTypes = authorizationService.getBannedStopPlaceTypes(stopPlace); + assertThat("Should contain banned StopPlaceType", bannedStopPlaceTypes.contains("airport"), is(false)); + boolean authorized = authorizationService.canEditEntity(roleAssignment, stopPlace); + assertThat("Should be authorized as both type and submode are allowed", authorized, is(true)); + + } + + private Polygon createPolygon(Point point) { + Geometry bufferedPoint = point.buffer(20); + LinearRing linearRing = new LinearRing(new CoordinateArraySequence(bufferedPoint.getCoordinates()), geometryFactory); + return geometryFactory.createPolygon(linearRing, null); } /**