check presence of user principal before checking ownership. fixes #80

clarin-eric · Mar 28, 2024 · f177c31 · f177c31
1 parent ed61a76
commit f177c31
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 24 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -2,6 +2,10 @@
 
 For upgrade instructions, see [UPGRADE.md](UPGRADE.md)
 
+## 
+- Fixed: error response on getting item rights information while unauthenticated
+	<https://github.com/clarin-eric/component-registry-rest/issues/80>
+
 ## Release Component Registry 2.5.1 (March 2024)
 -  Fixed: Team members cannot edit a published item with development status
 	<https://github.com/clarin-eric/component-registry-rest/issues/79>

diff --git a/...tRegistry/src/main/java/clarin/cmdi/componentregistry/impl/database/GroupServiceImpl.java b/...tRegistry/src/main/java/clarin/cmdi/componentregistry/impl/database/GroupServiceImpl.java
@@ -30,6 +30,7 @@
 import com.google.common.collect.Lists;
 import org.springframework.dao.DataAccessException;
 import clarin.cmdi.componentregistry.GroupService;
+import com.google.common.base.Strings;
 import com.google.common.collect.Streams;
 import java.util.stream.Stream;
 
@@ -108,32 +109,48 @@ public void removeOwnership(Ownership ownership) {
         throw new RuntimeException("not implemented");
     }
 
+    /**
+     * 
+     * @param principalName
+     * @param item
+     * @return if the user identified by the principal name either owns the
+     * item directly, or is a member of the group to which the item belongs.
+     */
     @Override
-    public boolean isUserOwnerEitherOnHisOwnOrThroughGroupMembership(String principalName, BaseDescription description) {
-        final RegistryUser user = userDao.getByPrincipalName(principalName);
+    public boolean isUserOwnerEitherOnHisOwnOrThroughGroupMembership(String principalName, BaseDescription item) {
+        if (Strings.isNullOrEmpty(principalName)) {
+            // no valid principal name -> cannot be owner
+            return false;
+        } else {
+            final RegistryUser user = userDao.getByPrincipalName(principalName);
+            if (user == null) {
+                // no such user -> cannot be owner
+                return false;
+            } else {
+                // TODO make some joins and multi-id queries to speed the entire method
+                // up
+                final long userId = user.getId();
+
+                // user is the owner
+                if (item.getUserId().equals(userId + "")) {
+                    return true;
+                }
 
-        // TODO make some joins and multi-id queries to speed the entire method
-        // up
-        final long userId = user.getId();
+                final String itemId = item.getId();
+                // a co-ownership on the profile also allows access
+                if (null != ownershipDao.findOwnershipByUserAndComponent(userId, itemId)) {
+                    return true;
+                }
 
-        // user is the owner
-        if (description.getUserId().equals(userId + "")) {
-            return true;
-        }
+                final Stream<Long> groupIdStreams = Streams.concat(
+                        groupDao.findGroupOwnedByUser(userId).stream().map(Group::getId),
+                        groupMembershipDao.findGroupsTheUserIsAmemberOf(userId).stream().map(GroupMembership::getGroupId));
 
-        final String itemId = description.getId();
-        // a co-ownership on the profile also allows access
-        if (null != ownershipDao.findOwnershipByUserAndComponent(userId, itemId)) {
-            return true;
+                // user is owner if the item is in group owned or a member of
+                return (groupIdStreams
+                        .anyMatch(groupId -> (null != ownershipDao.findOwnershipByGroupAndComponent(groupId, itemId))));
+            }
         }
-
-        final Stream<Long> groupIdStreams = Streams.concat(
-                groupDao.findGroupOwnedByUser(userId).stream().map(Group::getId),
-                groupMembershipDao.findGroupsTheUserIsAmemberOf(userId).stream().map(GroupMembership::getGroupId));
-
-        // user is owner if the item is in group owned or a member of
-         return (groupIdStreams
-                .anyMatch(groupId -> (null != ownershipDao.findOwnershipByGroupAndComponent(groupId, itemId))));
     }
 
     private boolean canUserAccessAbstractDescriptionEitherOnHisOwnOrThroughGroupMembership(RegistryUser user,

diff --git a/ComponentRegistry/src/main/java/clarin/cmdi/componentregistry/rest/ItemsService.java b/ComponentRegistry/src/main/java/clarin/cmdi/componentregistry/rest/ItemsService.java
@@ -41,6 +41,7 @@
 import org.springframework.stereotype.Service;
 import clarin.cmdi.componentregistry.GroupService;
 import clarin.cmdi.componentregistry.model.ItemRights;
+import com.google.common.base.Strings;
 import java.util.Optional;
 
 /**
@@ -110,7 +111,7 @@ public ItemRights getItemRights(@PathParam("itemId") String itemId) throws Compo
             final BaseDescription item = getItemDescriptionAccesControled(itemId);
             return itemRightsForPrincipal(principal, item);
         } catch (UserUnauthorizedException | AuthenticationRequiredException ex) {
-            return new ItemRights(itemId, principal == null ? "": principal.getName(), false, false);
+            return new ItemRights(itemId, principal == null ? "" : principal.getName(), false, false);
         } catch (ItemNotFoundException ex) {
             servletResponse.sendError(Response.Status.NOT_FOUND.getStatusCode(), "No such item");
             return null;
@@ -126,7 +127,7 @@ private ItemRights itemRightsForPrincipal(final Principal principal, final BaseD
 
         boolean canRead;
         boolean canWrite;
-        if (principal != null && isOwnerOrInOwningGroup(principal, item)) {
+        if (isOwnerOrInOwningGroup(principal, item)) {
             //(group) owner can always read
             canRead = true;
             //(group) owner can edit while in development status
@@ -139,7 +140,12 @@ private ItemRights itemRightsForPrincipal(final Principal principal, final BaseD
     }
 
     private boolean isOwnerOrInOwningGroup(Principal principal, BaseDescription item) {
-        return groupService.isUserOwnerEitherOnHisOwnOrThroughGroupMembership(principal.getName(), item);
+        if (principal == null) {
+            //no principal -> no ownership
+            return false;
+        } else {
+            return groupService.isUserOwnerEitherOnHisOwnOrThroughGroupMembership(principal.getName(), item);
+        }
     }
 
     @POST