Change UserFilter to only match if all given dimensions are a match

Author: hermannm

README.md

@@ -35,7 +35,7 @@ _Note that this repo only contains the User Roles service_
 ```
 {
   "username": "user123",
-  "userroles": [
+  "roles": [
     {
       "applicationName": "application1",
       "orgId": "orgId1",

src/main/kotlin/no/liflig/userroles/administration/UserFilter.kt

@@ -10,35 +10,35 @@ data class UserFilter(
     val applicationName: String?,
     val roleName: String?,
 ) {
+  /**
+   * Returns true if the given user role has a role that matches [orgId], [applicationName] and
+   * [roleName].
+   */
   fun matches(userRole: UserRole): Boolean {
+    /** Special case for no filter, to handle users with no roles. */
+    if (orgId == null && applicationName == null && roleName == null) {
+      return true
+    }
+
     /**
      * Super-admins match all orgId / applicationName filters, since they are implicitly part of all
      * orgs and applications. We only do this if no filter is provided for `roleName`.
      */
-    val matchAllOrganizationsAndApplications =
+    val matchAllOrgsAndApps =
         userRole.isSuperAdmin() && (this.roleName == null || this.roleName == SUPER_ADMIN_ROLE_NAME)
 
-    if (
-        orgId != null &&
-            userRole.roles.none { it.orgId == this.orgId } &&
-            !matchAllOrganizationsAndApplications
-    ) {
-      return false
-    }
+    return userRole.roles.any { role ->
+      val orgIdMatches = (orgId == null || role.orgId == orgId || matchAllOrgsAndApps)
 
-    if (
-        applicationName != null &&
-            userRole.roles.none { it.applicationName == this.applicationName } &&
-            !matchAllOrganizationsAndApplications
-    ) {
-      return false
-    }
+      val applicationMatches =
+          (applicationName == null ||
+              role.applicationName == applicationName ||
+              matchAllOrgsAndApps)
 
-    if (roleName != null && userRole.roles.none { it.roleName == this.roleName }) {
-      return false
-    }
+      val roleNameMatches = (roleName == null || role.roleName == roleName)
 
-    return true
+      orgIdMatches && applicationMatches && roleNameMatches
+    }
   }
 
   fun getCognitoFilterString(): String? {

src/test/kotlin/no/liflig/userroles/administration/UserFilterTest.kt

@@ -2,6 +2,7 @@ package no.liflig.userroles.administration
 
 import io.kotest.matchers.booleans.shouldBeFalse
 import io.kotest.matchers.booleans.shouldBeTrue
+import io.kotest.matchers.collections.shouldBeEmpty
 import no.liflig.userroles.testutils.DEFAULT_TEST_USERNAME
 import no.liflig.userroles.testutils.createRole
 import no.liflig.userroles.testutils.createUserRole
@@ -16,7 +17,7 @@ class UserFilterTest {
   fun `filter matches if any of the user's roles is a match`() {
     val userRole =
         createUserRole(
-            username = DEFAULT_TEST_USERNAME,
+            DEFAULT_TEST_USERNAME,
             createRole(orgId = "org2", applicationName = "app1", roleName = "role1"),
             createRole(orgId = "org1", applicationName = "app1", roleName = "role1"),
         )
@@ -25,11 +26,34 @@ class UserFilterTest {
     filter.matches(userRole).shouldBeTrue()
   }
 
+  @Test
+  fun `filter only matches if user has a role that fulfills all dimensions`() {
+    val filter = createUserFilter(orgId = "org1", applicationName = "app1", roleName = "role1")
+
+    val userRole =
+        createUserRole(
+            DEFAULT_TEST_USERNAME,
+            // Roles that match the filter in all dimensions except 1
+            createRole(orgId = "org1", applicationName = "app1", roleName = "role2"),
+            createRole(orgId = "org1", applicationName = "app2", roleName = "role1"),
+            createRole(orgId = "org2", applicationName = "app1", roleName = "role1"),
+        )
+    filter.matches(userRole).shouldBeFalse()
+
+    val matchingUserRole =
+        userRole.copy(
+            roles =
+                userRole.roles +
+                    createRole(orgId = "org1", applicationName = "app1", roleName = "role1")
+        )
+    filter.matches(matchingUserRole).shouldBeTrue()
+  }
+
   @Test
   fun `user with SUPER_ADMIN role matches all orgs and applications`() {
     val superAdmin =
         createUserRole(
-            username = DEFAULT_TEST_USERNAME,
+            DEFAULT_TEST_USERNAME,
             createRole(roleName = "SUPER_ADMIN", orgId = null, applicationName = null),
         )
 
@@ -45,6 +69,20 @@ class UserFilterTest {
     val appAndRoleFilter = createUserFilter(applicationName = "app1", roleName = "MEMBER")
     appAndRoleFilter.matches(superAdmin).shouldBeFalse()
   }
+
+  @Test
+  fun `empty filter always matches`() {
+    val emptyFilter = createUserFilter()
+
+    val userWithoutRoles = createUserRole()
+    userWithoutRoles.roles.shouldBeEmpty()
+
+    emptyFilter.matches(userWithoutRoles).shouldBeTrue()
+
+    val userWithRole = createUserRole(DEFAULT_TEST_USERNAME, createRole())
+
+    emptyFilter.matches(userWithRole).shouldBeTrue()
+  }
 }
 
 fun createUserFilter(