Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-835 Add permission-based access to warps #856

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,13 @@
import com.eternalcode.core.feature.afk.AfkSettings;
import com.eternalcode.core.feature.automessage.AutoMessageSettings;
import com.eternalcode.core.feature.chat.ChatSettings;
import com.eternalcode.core.feature.helpop.HelpOpSettings;
import com.eternalcode.core.feature.jail.JailSettings;
import com.eternalcode.core.feature.randomteleport.RandomTeleportSettings;
import com.eternalcode.core.feature.randomteleport.RandomTeleportType;
import com.eternalcode.core.feature.helpop.HelpOpSettings;
import com.eternalcode.core.feature.spawn.SpawnSettings;
import com.eternalcode.core.injector.annotations.component.ConfigurationFile;
import com.eternalcode.core.feature.teleportrequest.TeleportRequestSettings;
import java.util.LinkedHashMap;
import java.util.Set;
import com.eternalcode.core.injector.annotations.component.ConfigurationFile;
import net.dzikoysk.cdn.entity.Contextual;
import net.dzikoysk.cdn.entity.Description;
import net.dzikoysk.cdn.entity.Exclude;
Expand All @@ -25,7 +23,9 @@

import java.io.File;
import java.time.Duration;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;

@ConfigurationFile
public class PluginConfiguration implements ReloadableConfig {
Expand Down Expand Up @@ -396,6 +396,10 @@ public static class Warp {
@Description("# Texture of the item (only for PLAYER_HEAD material)")
public String itemTexture = "eyJ0ZXh0dXJlcyI6eyJTS0lOIjp7InVybCI6Imh0dHA6Ly90ZXh0dXJlcy5taW5lY3JhZnQubmV0L3RleHR1cmUvNzk4ODVlODIzZmYxNTkyNjdjYmU4MDkwOTNlMzNhNDc2ZTI3NDliNjU5OGNhNGEyYTgxZWU2OTczODAzZmI2NiJ9fX0=";

@Description("# Permissions assigned to warp")
public Map<String, Set<String>> warpPermissions = Map.of(
"default_warp", Set.of("eternalcore.warp.default")
);
}

@Description({ " ", "# Butcher" })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,16 @@
import dev.triumphteam.gui.builder.item.ItemBuilder;
import dev.triumphteam.gui.guis.Gui;
import dev.triumphteam.gui.guis.GuiItem;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import net.kyori.adventure.text.Component;
import net.kyori.adventure.text.minimessage.MiniMessage;
import org.bukkit.Material;
import org.bukkit.Server;
import org.bukkit.entity.Player;

import java.util.Collections;
import java.util.List;
import java.util.Optional;

@Service
public class WarpInventory {

Expand Down Expand Up @@ -198,13 +199,11 @@ public void openInventory(Player player, Language language) {
}

public void addWarp(Warp warp) {

if (!this.warpManager.warpExists(warp.getName())) {
return;
}

for (Language language : this.translationManager.getAvailableLanguages()) {

AbstractTranslation translation = (AbstractTranslation) this.translationManager.getMessages(language);
Translation.WarpSection.WarpInventorySection warpSection = translation.warp().warpInventory();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package com.eternalcode.core.feature.warp;

import com.eternalcode.annotations.scan.feature.FeatureDocs;
import com.eternalcode.core.configuration.implementation.PluginConfiguration;
import com.eternalcode.core.feature.warp.event.PreWarpTeleportEvent;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.Controller;
import com.eternalcode.core.notice.NoticeService;
import org.bukkit.entity.Player;
import org.bukkit.event.EventHandler;
import org.bukkit.event.Listener;

import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;

@FeatureDocs(
description = "Check if a player has permission to use a specific warp",
name = "Warp permission"
)
@Controller
public class WarpPermissionController implements Listener {

private final NoticeService noticeService;
private final PluginConfiguration pluginConfiguration;

@Inject
public WarpPermissionController(NoticeService noticeService, PluginConfiguration pluginConfiguration) {
this.noticeService = noticeService;
this.pluginConfiguration = pluginConfiguration;
}
Comment on lines +25 to +31
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove unused PluginConfiguration dependency

The pluginConfiguration field and its corresponding constructor parameter are declared but not used within the class. Removing unused fields helps keep the codebase clean and maintainable.

Apply this diff to remove the unused field and constructor parameter:

     private final NoticeService noticeService;
-    private final PluginConfiguration pluginConfiguration;

     @Inject
-    public WarpPermissionController(NoticeService noticeService, PluginConfiguration pluginConfiguration) {
+    public WarpPermissionController(NoticeService noticeService) {
         this.noticeService = noticeService;
-        this.pluginConfiguration = pluginConfiguration;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private final PluginConfiguration pluginConfiguration;
@Inject
public WarpPermissionController(NoticeService noticeService, PluginConfiguration pluginConfiguration) {
this.noticeService = noticeService;
this.pluginConfiguration = pluginConfiguration;
}
private final NoticeService noticeService;
@Inject
public WarpPermissionController(NoticeService noticeService) {
this.noticeService = noticeService;
}


@EventHandler
void onWarpPreTeleportation(PreWarpTeleportEvent event) {
Player player = event.getPlayer();
UUID uniqueId = player.getUniqueId();
Warp warp = event.getWarp();

this.checkWarpPermission(event, warp, player, uniqueId);
}

private void checkWarpPermission(PreWarpTeleportEvent event, Warp warp, Player player, UUID uniqueId) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Simplify method parameters by removing redundant UUID uniqueId

The uniqueId can be obtained from the player object within the checkWarpPermission method, reducing the number of parameters and simplifying the method signature.

Apply this diff to simplify the method:

-private void checkWarpPermission(PreWarpTeleportEvent event, Warp warp, Player player, UUID uniqueId) {
+private void checkWarpPermission(PreWarpTeleportEvent event, Warp warp, Player player) {

Update the usage of uniqueId within the method:

-    .player(uniqueId)
+    .player(player.getUniqueId())

And adjust the method call in onWarpPreTeleportation at line 39:

-this.checkWarpPermission(event, warp, player, uniqueId);
+this.checkWarpPermission(event, warp, player);

Committable suggestion was skipped due to low confidence.

Map<String, Set<String>> warpPermissions = this.pluginConfiguration.warp.warpPermissions;

if (!warpPermissions.containsKey(warp.getName())) {
return;
}

Set<String> permissions = warpPermissions.get(warp.getName());
Optional<String> isPlayerAllowedToUseWarp = permissions
.stream()
.filter(player::hasPermission)
.findAny();

if (isPlayerAllowedToUseWarp.isPresent()) {
return;
}

event.setCancelled(true);

this.noticeService.create()
.player(uniqueId)
.placeholder("{WARP}", warp.getName())
eripe14 marked this conversation as resolved.
Show resolved Hide resolved
.notice(translation -> translation.warp().noPermission())
.send();
Comment on lines +55 to +59
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance player feedback by including required permissions

Currently, the player is notified that they lack permission to use the warp but does not know which permission is needed. Providing the specific permissions required can help players understand what is needed to access the warp.

Consider modifying the notice to include the missing permissions:

 this.noticeService.create()
     .player(uniqueId)
     .placeholder("{WARP}", warp.getName())
+    .placeholder("{PERMISSIONS}", String.join(", ", permissions))
     .notice(translation -> translation.warp().noPermission())
     .send();

Ensure that the {PERMISSIONS} placeholder is appropriately handled in the noPermission() message template.

Committable suggestion was skipped due to low confidence.

}

}
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package com.eternalcode.core.translation;

import com.eternalcode.core.configuration.contextual.ConfigItem;
import com.eternalcode.core.feature.warp.WarpInventoryItem;
import com.eternalcode.core.feature.language.Language;
import com.eternalcode.core.feature.warp.WarpInventoryItem;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Missing Method Implementations in Translation Implementations

The following methods declared in the WarpSection interface are not implemented in the respective classes:

  • Class: ENTranslation.java

    • noPermission()
    • addPermissions()
    • removePermission()
    • permissionDoesNotExist()
    • noPermissionsProvided()
  • Class: PLTranslation.java

    • noPermission()
    • addPermissions()
    • removePermission()
    • permissionDoesNotExist()
    • noPermissionsProvided()

Please implement these methods to ensure full compliance with the WarpSection interface.

🔗 Analysis chain

Reactivated import statement

The import statement for WarpInventoryItem has been reactivated. This suggests that the WarpInventoryItem class is now being used within this file or its nested interfaces.

To ensure this import is necessary, let's verify its usage:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if WarpInventoryItem is used in the file
grep -n "WarpInventoryItem" eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java

Length of output: 2781

import com.eternalcode.multification.notice.Notice;
import org.bukkit.Material;
import org.bukkit.event.entity.EntityDamageEvent;

import java.util.Collection;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -146,6 +147,7 @@ interface WarpSection {
Notice itemAdded();
Notice noWarps();
Notice itemLimit();
Notice noPermission();

WarpInventorySection warpInventory();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
package com.eternalcode.core.translation.implementation;

import com.eternalcode.core.configuration.contextual.ConfigItem;
import com.eternalcode.core.feature.warp.WarpInventoryItem;
import com.eternalcode.core.feature.language.Language;
import com.eternalcode.core.feature.warp.WarpInventoryItem;
import com.eternalcode.core.translation.AbstractTranslation;
import com.eternalcode.multification.bukkit.notice.BukkitNotice;
import com.eternalcode.multification.notice.Notice;
import java.util.HashMap;
import lombok.Getter;
import lombok.experimental.Accessors;
import net.dzikoysk.cdn.entity.Contextual;
import net.dzikoysk.cdn.entity.Description;
import org.bukkit.Material;
import org.bukkit.Sound;
import org.bukkit.event.entity.EntityDamageEvent;

import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

Expand Down Expand Up @@ -370,6 +371,7 @@ public static class ENWarpSection implements WarpSection {
public Notice itemAdded = Notice.chat("<green>► <white>Warp has been added to GUI!");
public Notice noWarps = Notice.chat("<red>✘ <dark_red>There are no warps!");
public Notice itemLimit = Notice.chat("<red>✘ <dark_red>You have reached the limit of warps! Your limit is <red>{LIMIT}<dark_red>.");
public Notice noPermission = Notice.chat("<red>✘ <dark_red>You don't have permission to use this warp ({WARP})!");

@Description({" ", "# {WARPS} - List of warps (separated by commas)"})
public Notice available = Notice.chat("<green>► <white>Available warps: <green>{WARPS}");
Expand All @@ -382,7 +384,6 @@ public static class ENWarpSection implements WarpSection {
public static class ENWarpInventory implements WarpInventorySection {
public String title = "<dark_gray>» <green>Available warps:";


@Description({" ", "# Warps located inside GUI inventory can be customized here. More warps will be added on creation with /setwarp command. "})
public Map<String, WarpInventoryItem> items = new HashMap<>();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
package com.eternalcode.core.translation.implementation;

import com.eternalcode.core.configuration.contextual.ConfigItem;
import com.eternalcode.core.feature.warp.WarpInventoryItem;
import com.eternalcode.core.feature.language.Language;
import com.eternalcode.core.feature.warp.WarpInventoryItem;
import com.eternalcode.core.translation.AbstractTranslation;
import com.eternalcode.multification.bukkit.notice.BukkitNotice;
import com.eternalcode.multification.notice.Notice;
import java.util.HashMap;
import lombok.Getter;
import lombok.experimental.Accessors;
import net.dzikoysk.cdn.entity.Contextual;
import net.dzikoysk.cdn.entity.Description;
import org.bukkit.Material;
import org.bukkit.Sound;
import org.bukkit.event.entity.EntityDamageEvent;

import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

Expand Down Expand Up @@ -373,6 +374,7 @@ public static class PLWarpSection implements WarpSection {
public Notice itemAdded = Notice.chat("<green>► <white>Dodano warp do GUI!");
public Notice noWarps = Notice.chat("<red>✘ <dark_red>Błąd: <red>Nie ma dostępnych warpów!");
public Notice itemLimit = Notice.chat("<red>✘ <dark_red>Błąd: <red>Osiągnąłeś limit warpów w GUI! Limit to: {LIMIT}!");
public Notice noPermission = Notice.chat("<red>✘ <dark_red>Błąd: <red>Nie masz uprawnień do skorzystania z tego warpa ({WARP})!");
@Description({" ", "# {WARPS} - Lista dostępnych warpów"})
public Notice available = Notice.chat("<green>► <white>Dostepne warpy: <green>{WARPS}!");

Expand All @@ -395,7 +397,6 @@ public void setItems(Map<String, WarpInventoryItem> items) {
this.items = items;
}


public PLBorderSection border = new PLBorderSection();
public PLDecorationItemsSection decorationItems = new PLDecorationItemsSection();

Expand Down