From e84be5e4de7582bd8191bb2f492e76e2a8095fd9 Mon Sep 17 00:00:00 2001
From: ljacqu <ljacqu@users.noreply.github.com>
Date: Tue, 4 Dec 2018 19:45:46 +0100
Subject: [PATCH] #1702 Consider that Vault permission handler may return null
 for permission groups

---
 .../permission/handlers/VaultHandler.java     | 15 +++-
 .../permission/handlers/VaultHandlerTest.java | 80 +++++++++++++++++++
 2 files changed, 93 insertions(+), 2 deletions(-)
 create mode 100644 src/test/java/fr/xephi/authme/permission/handlers/VaultHandlerTest.java

diff --git a/src/main/java/fr/xephi/authme/permission/handlers/VaultHandler.java b/src/main/java/fr/xephi/authme/permission/handlers/VaultHandler.java
index 6e6815fe..dad2fba5 100644
--- a/src/main/java/fr/xephi/authme/permission/handlers/VaultHandler.java
+++ b/src/main/java/fr/xephi/authme/permission/handlers/VaultHandler.java
@@ -1,5 +1,6 @@
 package fr.xephi.authme.permission.handlers;
 
+import com.google.common.annotations.VisibleForTesting;
 import fr.xephi.authme.permission.PermissionNode;
 import fr.xephi.authme.permission.PermissionsSystemType;
 import net.milkbowl.vault.permission.Permission;
@@ -8,6 +9,7 @@ import org.bukkit.Server;
 import org.bukkit.plugin.RegisteredServiceProvider;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 /**
@@ -24,7 +26,15 @@ public class VaultHandler implements PermissionHandler {
         this.vaultProvider = getVaultPermission(server);
     }
 
-    private static Permission getVaultPermission(Server server) throws PermissionHandlerException {
+    /**
+     * Returns the Vault Permission interface.
+     *
+     * @param server the bukkit server instance
+     * @return the vault permission instance
+     * @throws PermissionHandlerException if the vault permission instance cannot be retrieved
+     */
+    @VisibleForTesting
+    Permission getVaultPermission(Server server) throws PermissionHandlerException {
         // Get the permissions provider service
         RegisteredServiceProvider<Permission> permissionProvider = server
             .getServicesManager().getRegistration(Permission.class);
@@ -76,7 +86,8 @@ public class VaultHandler implements PermissionHandler {
 
     @Override
     public List<String> getGroups(OfflinePlayer player) {
-        return Arrays.asList(vaultProvider.getPlayerGroups(null, player));
+        String[] groups = vaultProvider.getPlayerGroups(null, player);
+        return groups == null ? Collections.emptyList() : Arrays.asList(groups);
     }
 
     @Override
diff --git a/src/test/java/fr/xephi/authme/permission/handlers/VaultHandlerTest.java b/src/test/java/fr/xephi/authme/permission/handlers/VaultHandlerTest.java
new file mode 100644
index 00000000..006e865d
--- /dev/null
+++ b/src/test/java/fr/xephi/authme/permission/handlers/VaultHandlerTest.java
@@ -0,0 +1,80 @@
+package fr.xephi.authme.permission.handlers;
+
+import net.milkbowl.vault.permission.Permission;
+import org.bukkit.Server;
+import org.bukkit.entity.Player;
+import org.junit.Test;
+
+import java.util.List;
+
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.empty;
+import static org.junit.Assert.assertThat;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+/**
+ * Test for {@link VaultHandler}.
+ */
+public class VaultHandlerTest {
+
+    private VaultHandlerTestImpl vaultHandlerTest = VaultHandlerTestImpl.create();
+
+    @Test
+    public void shouldReturnGroups() {
+        // given
+        Permission permissionMock = vaultHandlerTest.permissionMock;
+        Player player = mock(Player.class);
+        given(permissionMock.getPlayerGroups(null, player)).willReturn(new String[]{"abc", "test"});
+
+        // when
+        List<String> result = vaultHandlerTest.getGroups(player);
+
+        // then
+        assertThat(result, contains("abc", "test"));
+        verify(permissionMock).getPlayerGroups(null, player);
+    }
+
+    /**
+     * Bug #1702: VaultHandler may return null for groups list.
+     */
+    @Test
+    public void shouldHandleNullAsGroups() {
+        // given
+        Permission permissionMock = vaultHandlerTest.permissionMock;
+        Player player = mock(Player.class);
+        given(permissionMock.getPlayerGroups(null, player)).willReturn(null);
+
+        // when
+        List<String> result = vaultHandlerTest.getGroups(player);
+
+        // then
+        assertThat(result, empty());
+        verify(permissionMock).getPlayerGroups(null, player);
+    }
+
+    /** Test implementation using a mock Vault Permission instance. */
+    private static final class VaultHandlerTestImpl extends VaultHandler {
+
+        private Permission permissionMock;
+
+        VaultHandlerTestImpl() throws PermissionHandlerException {
+            super(null);
+        }
+
+        static VaultHandlerTestImpl create() {
+            try {
+                return new VaultHandlerTestImpl();
+            } catch (PermissionHandlerException e) {
+                throw new IllegalStateException(e);
+            }
+        }
+
+        @Override
+        Permission getVaultPermission(Server server) {
+            permissionMock = mock(Permission.class);
+            return permissionMock;
+        }
+    }
+}