Make auto-op changes take effect (almost) immediately (#943)

"almost immediately" is better than "after a relog" ;)

bukkit/src/main/java/me/lucko/luckperms/bukkit/LPBukkitPlugin.java

@@ -27,6 +27,7 @@ package me.lucko.luckperms.bukkit;
 
 import me.lucko.luckperms.api.Contexts;
 import me.lucko.luckperms.api.LuckPermsApi;
+import me.lucko.luckperms.api.event.user.UserDataRecalculateEvent;
 import me.lucko.luckperms.bukkit.calculators.BukkitCalculatorFactory;
 import me.lucko.luckperms.bukkit.contexts.BukkitContextManager;
 import me.lucko.luckperms.bukkit.contexts.WorldCalculator;
@@ -44,6 +45,7 @@ import me.lucko.luckperms.bukkit.model.server.LPPermissionMap;
 import me.lucko.luckperms.bukkit.model.server.LPSubscriptionMap;
 import me.lucko.luckperms.bukkit.vault.VaultHookManager;
 import me.lucko.luckperms.common.api.LuckPermsApiProvider;
+import me.lucko.luckperms.common.api.delegates.model.ApiUser;
 import me.lucko.luckperms.common.calculators.PlatformCalculatorFactory;
 import me.lucko.luckperms.common.command.access.CommandPermission;
 import me.lucko.luckperms.common.config.ConfigKeys;
@@ -62,6 +64,7 @@ import me.lucko.luckperms.common.sender.Sender;
 import me.lucko.luckperms.common.tasks.CacheHousekeepingTask;
 import me.lucko.luckperms.common.tasks.ExpireTemporaryTask;
 
+import org.bukkit.OfflinePlayer;
 import org.bukkit.command.PluginCommand;
 import org.bukkit.entity.Player;
 import org.bukkit.permissions.Permission;
@@ -229,8 +232,22 @@ public class LPBukkitPlugin extends AbstractLuckPermsPlugin {
             // this throws an exception if the plugin is /reloaded, grr
         }
 
+        // remove all operators on startup if they're disabled
         if (!getConfiguration().get(ConfigKeys.OPS_ENABLED)) {
-            this.bootstrap.getScheduler().doSync(() -> this.bootstrap.getServer().getOperators().forEach(o -> o.setOp(false)));
+            this.bootstrap.getScheduler().platformAsync().execute(() -> {
+                for (OfflinePlayer player : this.bootstrap.getServer().getOperators()) {
+                    player.setOp(false);
+                }
+            });
+        }
+
+        // register autoop listener
+        if (getConfiguration().get(ConfigKeys.AUTO_OP)) {
+            getApiProvider().getEventBus().subscribe(UserDataRecalculateEvent.class, event -> {
+                User user = ApiUser.cast(event.getUser());
+                Optional<Player> player = getBootstrap().getPlayer(user.getUuid());
+                player.ifPresent(this::refreshAutoOp);
+            });
         }
 
         // replace the temporary executor when the Bukkit one starts
@@ -296,15 +313,16 @@ public class LPBukkitPlugin extends AbstractLuckPermsPlugin {
         }
     }
 
-    public void refreshAutoOp(User user, Player player) {
+    public void refreshAutoOp(Player player) {
-        if (user == null) {
-            return;
-        }
-
         if (getConfiguration().get(ConfigKeys.AUTO_OP)) {
-            Map<String, Boolean> backing = user.getCachedData().getPermissionData(this.contextManager.getApplicableContexts(player)).getImmutableBacking();
+            User user = getUserManager().getIfLoaded(player.getUniqueId());
-            boolean op = Optional.ofNullable(backing.get("luckperms.autoop")).orElse(false);
+            if (user == null) {
-            player.setOp(op);
+                player.setOp(false);
+                return;
+            }
+
+            Map<String, Boolean> permData = user.getCachedData().getPermissionData(this.contextManager.getApplicableContexts(player)).getImmutableBacking();
+            player.setOp(permData.getOrDefault("luckperms.autoop", false));
         }
     }
 

bukkit/src/main/java/me/lucko/luckperms/bukkit/listeners/BukkitConnectionListener.java

@@ -151,7 +151,7 @@ public class BukkitConnectionListener extends AbstractConnectionListener impleme
             t.printStackTrace();
         }
 
-        this.plugin.refreshAutoOp(user, player);
+        this.plugin.refreshAutoOp(player);
     }
 
     @EventHandler(priority = EventPriority.MONITOR)
@@ -160,24 +160,13 @@ public class BukkitConnectionListener extends AbstractConnectionListener impleme
            If the connection was cancelled here, we need to do something to clean up the data that was loaded. */
 
         // Check to see if this connection was denied at LOW. Even if it was denied at LOW, their data will still be present.
-        boolean denied = false;
         if (this.deniedLogin.remove(e.getPlayer().getUniqueId())) {
-            denied = true;
-
             // This is a problem, as they were denied at low priority, but are now being allowed.
             if (e.getResult() == PlayerLoginEvent.Result.ALLOWED) {
                 this.plugin.getLogger().severe("Player connection was re-allowed for " + e.getPlayer().getUniqueId());
                 e.disallow(PlayerLoginEvent.Result.KICK_OTHER, "");
             }
         }
-
-        // Login event was cancelled by another plugin since we first loaded their data
-        if (denied || e.getResult() != PlayerLoginEvent.Result.ALLOWED) {
-            return;
-        }
-
-        // everything is going well. login was processed ok, this is just to refresh auto-op status.
-        this.plugin.refreshAutoOp(this.plugin.getUserManager().getIfLoaded(e.getPlayer().getUniqueId()), e.getPlayer());
     }
 
     // Wait until the last priority to unload, so plugins can still perform permission checks on this event

bukkit/src/main/java/me/lucko/luckperms/bukkit/listeners/BukkitPlatformListener.java

@@ -95,7 +95,7 @@ public class BukkitPlatformListener implements Listener {
     @EventHandler(priority = EventPriority.LOWEST)
     public void onWorldChange(PlayerChangedWorldEvent e) {
         this.plugin.getContextManager().invalidateCache(e.getPlayer());
-        this.plugin.refreshAutoOp(this.plugin.getUserManager().getIfLoaded(e.getPlayer().getUniqueId()), e.getPlayer());
+        this.plugin.refreshAutoOp(e.getPlayer());
     }
 
 }

bukkit/src/main/resources/config.yml

@@ -468,9 +468,6 @@ enable-ops: true
 # - Additionally, setting this to true will force the "enable-ops" option above to false. All users
 #   will be de-opped unless they have the permission node, and the op/deop commands will be
 #   disabled.
-# - It is important to note that this setting is only checked when a player first joins the server,
-#   and when they switch worlds. Therefore, simply removing this permission from a user will not
-#   automatically de-op them. A player needs to relog to have the change take effect.
 # - It is recommended that you use this option instead of assigning a single '*' permission.
 auto-op: false
 

nukkit/src/main/java/me/lucko/luckperms/nukkit/LPNukkitPlugin.java

@@ -27,7 +27,9 @@ package me.lucko.luckperms.nukkit;
 
 import me.lucko.luckperms.api.Contexts;
 import me.lucko.luckperms.api.LuckPermsApi;
+import me.lucko.luckperms.api.event.user.UserDataRecalculateEvent;
 import me.lucko.luckperms.common.api.LuckPermsApiProvider;
+import me.lucko.luckperms.common.api.delegates.model.ApiUser;
 import me.lucko.luckperms.common.calculators.PlatformCalculatorFactory;
 import me.lucko.luckperms.common.command.access.CommandPermission;
 import me.lucko.luckperms.common.config.ConfigKeys;
@@ -202,11 +204,21 @@ public class LPNukkitPlugin extends AbstractLuckPermsPlugin {
             // this throws an exception if the plugin is /reloaded, grr
         }
 
+        // remove all operators on startup if they're disabled
         if (!getConfiguration().get(ConfigKeys.OPS_ENABLED)) {
             Config ops = this.bootstrap.getServer().getOps();
             ops.getKeys(false).forEach(ops::remove);
         }
 
+        // register autoop listener
+        if (getConfiguration().get(ConfigKeys.AUTO_OP)) {
+            getApiProvider().getEventBus().subscribe(UserDataRecalculateEvent.class, event -> {
+                User user = ApiUser.cast(event.getUser());
+                Optional<Player> player = getBootstrap().getPlayer(user.getUuid());
+                player.ifPresent(this::refreshAutoOp);
+            });
+        }
+
         // replace the temporary executor when the Nukkit one starts
         this.bootstrap.getServer().getScheduler().scheduleTask(this.bootstrap, () -> this.bootstrap.getScheduler().setUseFallback(false), true);
 
@@ -265,15 +277,16 @@ public class LPNukkitPlugin extends AbstractLuckPermsPlugin {
         InjectorDefaultsMap.uninject();
     }
 
-    public void refreshAutoOp(User user, Player player) {
+    public void refreshAutoOp(Player player) {
-        if (user == null) {
-            return;
-        }
-
         if (getConfiguration().get(ConfigKeys.AUTO_OP)) {
-            Map<String, Boolean> backing = user.getCachedData().getPermissionData(this.contextManager.getApplicableContexts(player)).getImmutableBacking();
+            User user = getUserManager().getIfLoaded(player.getUniqueId());
-            boolean op = Optional.ofNullable(backing.get("luckperms.autoop")).orElse(false);
+            if (user == null) {
-            player.setOp(op);
+                player.setOp(false);
+                return;
+            }
+
+            Map<String, Boolean> permData = user.getCachedData().getPermissionData(this.contextManager.getApplicableContexts(player)).getImmutableBacking();
+            player.setOp(permData.getOrDefault("luckperms.autoop", false));
         }
     }
 

nukkit/src/main/java/me/lucko/luckperms/nukkit/listeners/NukkitConnectionListener.java

@@ -143,7 +143,7 @@ public class NukkitConnectionListener extends AbstractConnectionListener impleme
             t.printStackTrace();
         }
 
-        this.plugin.refreshAutoOp(user, player);
+        this.plugin.refreshAutoOp(player);
     }
 
     @EventHandler(priority = EventPriority.MONITOR)
@@ -152,24 +152,13 @@ public class NukkitConnectionListener extends AbstractConnectionListener impleme
            If the connection was cancelled here, we need to do something to clean up the data that was loaded. */
 
         // Check to see if this connection was denied at LOW. Even if it was denied at LOW, their data will still be present.
-        boolean denied = false;
         if (this.deniedLogin.remove(e.getPlayer().getUniqueId())) {
-            denied = true;
-
             // This is a problem, as they were denied at low priority, but are now being allowed.
             if (!e.isCancelled()) {
                 this.plugin.getLogger().severe("Player connection was re-allowed for " + e.getPlayer().getUniqueId());
                 e.setCancelled();
             }
         }
-
-        // Login event was cancelled by another plugin since we first loaded their data
-        if (denied || e.isCancelled()) {
-            return;
-        }
-
-        // everything is going well. login was processed ok, this is just to refresh auto-op status.
-        this.plugin.refreshAutoOp(this.plugin.getUserManager().getIfLoaded(e.getPlayer().getUniqueId()), e.getPlayer());
     }
 
     // Wait until the last priority to unload, so plugins can still perform permission checks on this event

nukkit/src/main/java/me/lucko/luckperms/nukkit/listeners/NukkitPlatformListener.java

@@ -90,7 +90,7 @@ public class NukkitPlatformListener implements Listener {
         if (e.getEntity() instanceof Player) {
             Player player = (Player) e.getEntity();
             this.plugin.getContextManager().invalidateCache(player);
-            this.plugin.refreshAutoOp(this.plugin.getUserManager().getIfLoaded(player.getUniqueId()), player);
+            this.plugin.refreshAutoOp(player);
         }
     }
 

nukkit/src/main/resources/config.yml

@@ -463,9 +463,6 @@ enable-ops: true
 # - Additionally, setting this to true will force the "enable-ops" option above to false. All users
 #   will be de-opped unless they have the permission node, and the op/deop commands will be
 #   disabled.
-# - It is important to note that this setting is only checked when a player first joins the server,
-#   and when they switch worlds. Therefore, simply removing this permission from a user will not
-#   automatically de-op them. A player needs to relog to have the change take effect.
 # - It is recommended that you use this option instead of assigning a single '*' permission.
 auto-op: false