From c12f5fa17952955d81b1231328a6f278997e74c0 Mon Sep 17 00:00:00 2001 From: Francois Prunayre Date: Wed, 30 Dec 2020 14:52:52 +0100 Subject: [PATCH] Security / Build permission filter based on user info. --- modules/library/README.md | 12 ++++ .../common/search/ElasticSearchProxy.java | 12 +++- .../geonet/common/search/FilterBuilder.java | 12 ++-- .../geonet/common/search/domain/UserInfo.java | 13 ++-- ...ndSelectionAwareResponseProcessorImpl.java | 2 +- modules/services/authorizing/README.md | 17 +++++ .../authorizing/GnUserDetailsService.java | 11 ++-- .../authorizing/GnUserDetailServiceTest.java | 11 +++- .../controller/SecurityTesterController.java | 21 ++++-- .../src/main/resources/bootstrap.yml | 2 +- .../common/search/FilterBuilderTest.java | 65 +++++++++++++++++++ 11 files changed, 147 insertions(+), 31 deletions(-) create mode 100644 modules/services/searching/src/test/java/org/fao/geonet/common/search/FilterBuilderTest.java diff --git a/modules/library/README.md b/modules/library/README.md index 629d9e8e..8d091767 100644 --- a/modules/library/README.md +++ b/modules/library/README.md @@ -3,6 +3,18 @@ Slf4j is used as a logging facade, and by default the logs will be redirected onto the standard output. + +Create a topic in the class and use the log: +```java +@Slf4j(topic = "org.fao.geonet.indexing.tasks") +public class IndexingService { + ... + log.info(String.format( + "Index %s removed.", + index)); +``` + + If one need to tweak the log level, it can be done by modifying the `bootstrap.yml` file of the spring-boot application, e.g.: diff --git a/modules/library/common-search/src/main/java/org/fao/geonet/common/search/ElasticSearchProxy.java b/modules/library/common-search/src/main/java/org/fao/geonet/common/search/ElasticSearchProxy.java index ef3a4a36..5d731ea5 100644 --- a/modules/library/common-search/src/main/java/org/fao/geonet/common/search/ElasticSearchProxy.java +++ b/modules/library/common-search/src/main/java/org/fao/geonet/common/search/ElasticSearchProxy.java @@ -669,10 +669,18 @@ private UserInfo getUserInfo() { GrantedAuthority authority = authentication.getAuthorities().stream().findFirst().get(); if (authority.getAuthority().equals("gn")) { OAuth2UserAuthority userAuthority = (OAuth2UserAuthority) authority; + Map attributes = userAuthority.getAttributes(); + + String highestProfile = (String) attributes.get("highest_profile"); + if (StringUtils.isNotEmpty(highestProfile)) { + userInfo.setHighestProfile(Profile.valueOf(highestProfile).name()); + } + + userInfo.setUserId((Integer) attributes.get("user_id")); userInfo.setViewingGroups( - (List) userAuthority.getAttributes().get(Profile.Reviewer.name())); + (List) attributes.get(Profile.Reviewer.name())); userInfo.setEditingGroups( - (List) userAuthority.getAttributes().get(Profile.Editor.name())); + (List) attributes.get(Profile.Editor.name())); } return userInfo; diff --git a/modules/library/common-search/src/main/java/org/fao/geonet/common/search/FilterBuilder.java b/modules/library/common-search/src/main/java/org/fao/geonet/common/search/FilterBuilder.java index ac38926c..df7b2ac0 100644 --- a/modules/library/common-search/src/main/java/org/fao/geonet/common/search/FilterBuilder.java +++ b/modules/library/common-search/src/main/java/org/fao/geonet/common/search/FilterBuilder.java @@ -59,7 +59,7 @@ public String buildQueryFilter(String type, UserInfo userInfo) { */ private String buildPermissionsFilter(UserInfo userInfo) { // If admin you can see all - if (Profile.Administrator.equals(userInfo.getProfile())) { + if (Profile.Administrator.name().equals(userInfo.getHighestProfile())) { return "*"; } else { // op0 (ie. view operation) contains one of the ids of your groups @@ -68,15 +68,15 @@ private String buildPermissionsFilter(UserInfo userInfo) { .collect(Collectors.joining(" OR ")); String operationFilter = String.format("op%d:(%s)", ReservedOperation.view.getId(), ids); - - String ownerFilter = ""; - if (userInfo.isAuthenticated()) { + if (userInfo.isAuthenticated() && userInfo.getUserId() != null) { // OR you are owner - ownerFilter = String.format("owner:%d", userInfo.getUserId()); + String ownerFilter = String.format("owner:%d", userInfo.getUserId()); // OR member of groupOwner // TODOES + return String.format("(%s %s)", operationFilter, ownerFilter); + } else { + return operationFilter; } - return String.format("(%s %s)", operationFilter, ownerFilter).trim(); } } diff --git a/modules/library/common-search/src/main/java/org/fao/geonet/common/search/domain/UserInfo.java b/modules/library/common-search/src/main/java/org/fao/geonet/common/search/domain/UserInfo.java index b8df68f6..a72ffbef 100644 --- a/modules/library/common-search/src/main/java/org/fao/geonet/common/search/domain/UserInfo.java +++ b/modules/library/common-search/src/main/java/org/fao/geonet/common/search/domain/UserInfo.java @@ -23,6 +23,10 @@ public class UserInfo { @Setter private String userName; + @Getter + @Setter + private String highestProfile; + @Getter @Setter private List viewingGroups = new ArrayList<>(); @@ -41,15 +45,6 @@ public boolean isAuthenticated() { && (!userName.equalsIgnoreCase("anonymousUser"))); } - /** - * Retrieve the user profile. - * - * @return User profile. - */ - public String getProfile() { - return ""; - } - /** * Retrieve the user groups. * diff --git a/modules/library/common-search/src/main/java/org/fao/geonet/common/search/processor/impl/JsonUserAndSelectionAwareResponseProcessorImpl.java b/modules/library/common-search/src/main/java/org/fao/geonet/common/search/processor/impl/JsonUserAndSelectionAwareResponseProcessorImpl.java index 3de94cb6..5b09b032 100644 --- a/modules/library/common-search/src/main/java/org/fao/geonet/common/search/processor/impl/JsonUserAndSelectionAwareResponseProcessorImpl.java +++ b/modules/library/common-search/src/main/java/org/fao/geonet/common/search/processor/impl/JsonUserAndSelectionAwareResponseProcessorImpl.java @@ -240,7 +240,7 @@ private boolean isOwner(UserInfo userInfo, Integer owner, Integer groupOwner) { } //--- check if the user is an administrator - final Profile profile = Profile.findProfileIgnoreCase(userInfo.getProfile()); + final Profile profile = Profile.findProfileIgnoreCase(userInfo.getHighestProfile()); if (profile == Profile.Administrator) { return true; } diff --git a/modules/services/authorizing/README.md b/modules/services/authorizing/README.md index 9a2f138e..e2692bcc 100644 --- a/modules/services/authorizing/README.md +++ b/modules/services/authorizing/README.md @@ -15,6 +15,8 @@ GeoNetwork does not need to be started, but it MUST have started once to create 1. Get an authentication token ```shell script +USERNAME=editor +PASSWORD=aaaaaa USERNAME=admin PASSWORD=admin gn_token=$( \ @@ -42,8 +44,23 @@ echo $gn_token 3. The token is used to access services ```shell script +docker-compose up -d search + gn_auth_header=$(echo "Authorization: Bearer $gn_token") curl 127.0.0.1:9900/search/secured -H "$gn_auth_header" ``` +The server return user details: + +``` +curl 127.0.0.1:9900/search/secured -H "$gn_auth_header" +You are authenticated as editor +Authorities gn + * UserAdmin:[] + * highest_profile:Editor + * Editor:[100] + * Reviewer:[] + * RegisteredUser:[101] +``` + diff --git a/modules/services/authorizing/src/main/java/org/fao/geonet/authorizing/GnUserDetailsService.java b/modules/services/authorizing/src/main/java/org/fao/geonet/authorizing/GnUserDetailsService.java index e1b030f5..c08407a7 100644 --- a/modules/services/authorizing/src/main/java/org/fao/geonet/authorizing/GnUserDetailsService.java +++ b/modules/services/authorizing/src/main/java/org/fao/geonet/authorizing/GnUserDetailsService.java @@ -27,6 +27,9 @@ import org.springframework.security.oauth2.core.user.OAuth2UserAuthority; public class GnUserDetailsService implements UserDetailsService { + public static final String HIGHEST_PROFILE = "highest_profile"; + public static final String USER_ID = "user_id"; + public static final String GN_AUTHORITY = "gn"; @Autowired private UserRepository userRepository; @@ -34,9 +37,6 @@ public class GnUserDetailsService implements UserDetailsService { @Autowired private UserGroupRepository userGroupRepository; - @Autowired - private GroupRepository groupRepository; - private boolean checkUserNameOrEmail = true; @Override @@ -61,12 +61,13 @@ public UserDetails loadUserByUsername(String userNameOrEmail) throws UsernameNot Collectors.mapping(t -> t.getValue(), Collectors.toList()) )); Map attributes = (Map) (Object) attributesToCast; - attributes.put("highest_profile", user.getProfile().name()); + attributes.put(USER_ID, user.getId()); + attributes.put(HIGHEST_PROFILE, user.getProfile().name()); attributes.putIfAbsent(Profile.UserAdmin.name(), Collections.emptyList()); attributes.putIfAbsent(Profile.Reviewer.name(), Collections.emptyList()); attributes.putIfAbsent(Profile.RegisteredUser.name(), Collections.emptyList()); attributes.putIfAbsent(Profile.Editor.name(), Collections.emptyList()); - OAuth2UserAuthority authority = new OAuth2UserAuthority("gn", attributes); + OAuth2UserAuthority authority = new OAuth2UserAuthority(GN_AUTHORITY, attributes); return org.springframework.security.core.userdetails.User .withUsername(user.getUsername()) diff --git a/modules/services/authorizing/src/test/java/org/fao/geonet/authorizing/GnUserDetailServiceTest.java b/modules/services/authorizing/src/test/java/org/fao/geonet/authorizing/GnUserDetailServiceTest.java index 08065a9a..3905a30a 100644 --- a/modules/services/authorizing/src/test/java/org/fao/geonet/authorizing/GnUserDetailServiceTest.java +++ b/modules/services/authorizing/src/test/java/org/fao/geonet/authorizing/GnUserDetailServiceTest.java @@ -1,5 +1,7 @@ package org.fao.geonet.authorizing; +import static org.fao.geonet.authorizing.GnUserDetailsService.HIGHEST_PROFILE; +import static org.fao.geonet.authorizing.GnUserDetailsService.USER_ID; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -45,10 +47,12 @@ public void nominal() { Group group = new Group(); group.setName("csc"); groupRepository.save(group); + User user = new User(); user.setUsername("csc_editor"); user.setProfile(Profile.Editor); userRepository.save(user); + UserGroup userGroup = new UserGroup(); userGroup.setGroup(group); userGroup.setUser(user); @@ -58,17 +62,20 @@ public void nominal() { UserDetails userDetails = toTest.loadUserByUsername("csc_editor"); assertEquals("csc_editor", userDetails.getUsername()); + assertEquals(1, userDetails.getAuthorities().size()); assertTrue(userDetails.getAuthorities().stream().findFirst().get() instanceof OAuth2UserAuthority); OAuth2UserAuthority authority = (OAuth2UserAuthority) userDetails.getAuthorities().stream().findFirst().get(); assertEquals("gn", authority.getAuthority()); + Map attributes = authority.getAttributes(); - assertEquals(5, attributes.size()); + assertEquals(6, attributes.size()); assertEquals(Arrays.asList(group.getId()), attributes.get(Profile.Editor.name())); assertEquals(Collections.emptyList(), attributes.get(Profile.Reviewer.name())); assertEquals(Collections.emptyList(), attributes.get(Profile.UserAdmin.name())); assertEquals(Collections.emptyList(), attributes.get(Profile.RegisteredUser.name())); - assertEquals(Profile.Editor.name(), attributes.get("highest_profile")); + assertEquals(Profile.Editor.name(), attributes.get(HIGHEST_PROFILE)); + assertEquals(100, attributes.get(USER_ID)); } @Configuration diff --git a/modules/services/searching/src/main/java/org/fao/geonet/searching/controller/SecurityTesterController.java b/modules/services/searching/src/main/java/org/fao/geonet/searching/controller/SecurityTesterController.java index 4d8612f8..7e4fef01 100644 --- a/modules/services/searching/src/main/java/org/fao/geonet/searching/controller/SecurityTesterController.java +++ b/modules/services/searching/src/main/java/org/fao/geonet/searching/controller/SecurityTesterController.java @@ -6,10 +6,12 @@ package org.fao.geonet.searching.controller; import java.security.Principal; +import java.util.Map; import java.util.Optional; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.annotation.AuthenticationPrincipal; +import org.springframework.security.oauth2.core.user.OAuth2UserAuthority; import org.springframework.security.oauth2.provider.OAuth2Authentication; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; @@ -18,7 +20,7 @@ public class SecurityTesterController { /** - * A simple secured endpoint returning username. + * A simple secured endpoint returning user details stored in JWT. */ @RequestMapping("/search/secured") public String search( @@ -30,9 +32,18 @@ public String search( Optional authority = oauth2Authentication.getAuthorities() .stream().findFirst(); - return String.format( - "Search service called. You are authenticated as %s. Authorities: %s", - name, - authority.isPresent() ? authority.get().getAuthority() : ""); + if (authority.isPresent() && authority.get().getAuthority().equals("gn")) { + OAuth2UserAuthority oauthAuthority = (OAuth2UserAuthority) authority.get(); + Map attributes = oauthAuthority.getAttributes(); + StringBuilder message = new StringBuilder(); + message.append(String.format("You are authenticated as %s\n", name)); + message.append(String.format("Authorities %s\n", oauthAuthority.getAuthority())); + attributes.forEach((k, v) -> { + message.append(" * ").append(k).append(":").append(v).append("\n"); + }); + return message.toString(); + } else { + return String.format("No authority found. User is %s", name); + } } } diff --git a/modules/services/searching/src/main/resources/bootstrap.yml b/modules/services/searching/src/main/resources/bootstrap.yml index 8160f4dd..ba2031c0 100644 --- a/modules/services/searching/src/main/resources/bootstrap.yml +++ b/modules/services/searching/src/main/resources/bootstrap.yml @@ -33,7 +33,7 @@ eureka: logging: level: org.springframework.security: DEBUG - + org.fao.geonet.searching: DEBUG --- # Use this profile when running on a GN4 database on localhost and Elasticsearch index. diff --git a/modules/services/searching/src/test/java/org/fao/geonet/common/search/FilterBuilderTest.java b/modules/services/searching/src/test/java/org/fao/geonet/common/search/FilterBuilderTest.java new file mode 100644 index 00000000..48101262 --- /dev/null +++ b/modules/services/searching/src/test/java/org/fao/geonet/common/search/FilterBuilderTest.java @@ -0,0 +1,65 @@ +/** + * (c) 2020 Open Source Geospatial Foundation - all rights reserved This code is licensed under the + * GPL 2.0 license, available at the root application directory. + */ + +package org.fao.geonet.common.search; + +import org.fao.geonet.common.search.domain.Profile; +import org.fao.geonet.common.search.domain.UserInfo; +import org.junit.Before; +import org.junit.Test; + +import java.util.Arrays; + +import static org.junit.jupiter.api.Assertions.assertTrue; + + +public class FilterBuilderTest { + + FilterBuilder filterBuilder; + + @Before + public void setup() { + this.filterBuilder = new FilterBuilder(); + } + + @Test + public void testFilterType() { + UserInfo userInfo = new UserInfo(); + String queryFilter = filterBuilder.buildQueryFilter("", userInfo); + assertTrue(queryFilter.contains("op0:(1) AND (draft:n OR draft:e)")); + queryFilter = filterBuilder.buildQueryFilter("metadata", userInfo); + assertTrue(queryFilter.contains("op0:(1) AND (isTemplate:n) AND (draft:n OR draft:e)")); + queryFilter = filterBuilder.buildQueryFilter("template", userInfo); + assertTrue(queryFilter.contains("op0:(1) AND (isTemplate:y) AND (draft:n OR draft:e)")); + queryFilter = filterBuilder.buildQueryFilter("subtemplate", userInfo); + assertTrue(queryFilter.contains("op0:(1) AND (isTemplate:s) AND (draft:n OR draft:e)")); + } + + @Test + public void testFilterForAnonymousUser() { + UserInfo userInfo = new UserInfo(); + String queryFilter = filterBuilder.buildQueryFilter("metadata", userInfo); + assertTrue(queryFilter.contains("op0:(1) AND (isTemplate:n) AND (draft:n OR draft:e)")); + } + + @Test + public void testFilterForAdministrator() { + UserInfo userInfo = new UserInfo(); + userInfo.setHighestProfile(Profile.Administrator.name()); + String queryFilter = filterBuilder.buildQueryFilter("metadata", userInfo); + assertTrue(queryFilter.contains("* AND (isTemplate:n) AND (draft:n OR draft:e)")); + } + + @Test + public void testFilterForEditor() { + UserInfo userInfo = new UserInfo(); + userInfo.setUserName("Dad"); + userInfo.setUserId(999); + userInfo.setHighestProfile(Profile.Editor.name()); + userInfo.setEditingGroups(Arrays.asList(new Integer[]{10, 11, 12})); + String queryFilter = filterBuilder.buildQueryFilter("metadata", userInfo); + assertTrue(queryFilter.contains("(op0:(1 OR 10 OR 11 OR 12) owner:999) AND (isTemplate:n) AND (draft:n OR draft:e)")); + } +} \ No newline at end of file