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

[Feature] 通过 WebUI 编辑配置文件 #513

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
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
17 changes: 12 additions & 5 deletions src/main/java/com/ghostchu/peerbanhelper/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@
import com.ghostchu.peerbanhelper.util.PBHLibrariesLoader;
import com.ghostchu.peerbanhelper.util.Slf4jLogAppender;
import com.ghostchu.simplereloadlib.ReloadManager;
import com.ghostchu.simplereloadlib.ReloadResult;
import com.ghostchu.simplereloadlib.ReloadableContainer;
import com.google.common.eventbus.EventBus;
import com.google.common.io.ByteStreams;
import lombok.Getter;
import lombok.extern.slf4j.Slf4j;
import org.apache.logging.log4j.core.config.plugins.util.PluginManager;
import org.bspfsystems.yamlconfiguration.configuration.InvalidConfigurationException;
import org.bspfsystems.yamlconfiguration.file.YamlConfiguration;
import org.jetbrains.annotations.Nullable;
Expand Down Expand Up @@ -103,7 +104,7 @@ public static void main(String[] args) {
DEF_LOCALE = mainConfig.getString("language");
if (DEF_LOCALE == null || DEF_LOCALE.equalsIgnoreCase("default")) {
DEF_LOCALE = System.getenv("PBH_USER_LOCALE");
if(DEF_LOCALE == null) {
if (DEF_LOCALE == null) {
DEF_LOCALE = Locale.getDefault().toLanguageTag();
}
}
Expand Down Expand Up @@ -165,9 +166,9 @@ private static void setupConfDirectory(String[] args) {
root = new File(System.getenv("LOCALAPPDATA"), "PeerBanHelper").getAbsolutePath();
} else {
var dataDirectory = new File(System.getProperty("user.home")).toPath();
if(osName.contains("mac")){
if (osName.contains("mac")) {
dataDirectory = dataDirectory.resolve("/Library/Application Support");
}else{
} else {
dataDirectory = dataDirectory.resolve(".config");
}
root = dataDirectory.resolve("PeerBanHelper").toAbsolutePath().toString();
Expand Down Expand Up @@ -342,7 +343,7 @@ public static String decapitalize(String name) {
return name;
}
if (name.length() > 1 && Character.isUpperCase(name.charAt(1)) &&
Character.isUpperCase(name.charAt(0))) {
Character.isUpperCase(name.charAt(0))) {
return name;
}
char chars[] = name.toCharArray();
Expand Down Expand Up @@ -391,4 +392,10 @@ public static void unregisterBean(String beanName) {
defaultListableBeanFactory.removeBeanDefinition(beanName);
}

public static Map<ReloadableContainer, ReloadResult> reloadConfig() {
mainConfig = loadConfiguration(mainConfigFile);
profileConfig = loadConfiguration(profileConfigFile);
setupProxySettings();
return reloadManager.reload();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,7 @@ private void registerModules() {
moduleManager.register(PBHGeneralController.class);
moduleManager.register(PBHTorrentController.class);
moduleManager.register(PBHPeerController.class);
moduleManager.register(WebConfigController.class);
}

public Map<Downloader, Map<Torrent, List<Peer>>> collectPeers() {
Expand Down
10 changes: 9 additions & 1 deletion src/main/java/com/ghostchu/peerbanhelper/btn/BtnConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

import com.ghostchu.peerbanhelper.PeerBanHelperServer;
import com.ghostchu.peerbanhelper.text.Lang;
import com.ghostchu.simplereloadlib.ReloadResult;
import com.ghostchu.simplereloadlib.ReloadStatus;
import com.ghostchu.simplereloadlib.Reloadable;
import lombok.extern.slf4j.Slf4j;
import org.bspfsystems.yamlconfiguration.configuration.ConfigurationSection;
import org.springframework.beans.factory.annotation.Autowired;
Expand All @@ -13,7 +16,7 @@

@Configuration
@Slf4j
public class BtnConfig {
public class BtnConfig implements Reloadable {
@Autowired
private PeerBanHelperServer server;
@Autowired
Expand All @@ -37,4 +40,9 @@ public BtnNetwork btnNetwork() {
return null;
}
}

@Override
public ReloadResult reloadModule() throws Exception {
return ReloadResult.builder().status(ReloadStatus.REQUIRE_RESTART).build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.ghostchu.peerbanhelper.util.rule.matcher.IPMatcher;
import com.ghostchu.peerbanhelper.web.wrapper.StdResp;
import com.ghostchu.simplereloadlib.ReloadResult;
import com.ghostchu.simplereloadlib.ReloadStatus;
import com.ghostchu.simplereloadlib.Reloadable;
import com.github.mizosoft.methanol.MutableRequest;
import com.google.common.hash.HashCode;
Expand Down Expand Up @@ -108,8 +109,8 @@ public void onDisable() {

@Override
public ReloadResult reloadModule() throws Exception {
reloadConfig();
return Reloadable.super.reloadModule();
Thread.ofVirtual().start(this::reloadConfig);
return ReloadResult.builder().status(ReloadStatus.SCHEDULED).build();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
package com.ghostchu.peerbanhelper.module.impl.webapi;

import com.ghostchu.peerbanhelper.Main;
import com.ghostchu.peerbanhelper.module.AbstractFeatureModule;
import com.ghostchu.peerbanhelper.util.context.IgnoreScan;
import com.ghostchu.peerbanhelper.web.JavalinWebContainer;
import com.ghostchu.peerbanhelper.web.Role;
import com.ghostchu.peerbanhelper.web.wrapper.StdResp;
import com.ghostchu.simplereloadlib.ReloadResult;
import com.ghostchu.simplereloadlib.ReloadableContainer;
import io.javalin.http.Context;
import org.bspfsystems.yamlconfiguration.configuration.InvalidConfigurationException;
import org.bspfsystems.yamlconfiguration.file.YamlConfiguration;
import org.jetbrains.annotations.NotNull;
import org.springframework.stereotype.Component;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;

@Component
@IgnoreScan
public class WebConfigController extends AbstractFeatureModule {
private final JavalinWebContainer javalinWebContainer;

public WebConfigController(JavalinWebContainer javalinWebContainer) {
super();
this.javalinWebContainer = javalinWebContainer;
}

@Override
public boolean isConfigurable() {
return false;
}

@Override
public @NotNull String getName() {
return "Web Config Editor";
}

@Override
public @NotNull String getConfigName() {
return "web-config-editor";
}

@Override
public void onEnable() {
javalinWebContainer.javalin()
.get("/api/config", this::listConfig, Role.USER_READ)
.get("/api/config/{configId}", this::retrieveConfigContent, Role.USER_WRITE)
.put("/api/config/{configId}", this::editConfig, Role.USER_WRITE);
Comment on lines +51 to +54
Copy link
Contributor

Choose a reason for hiding this comment

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

Potential security risk: Exposing configuration files via API

Allowing users to retrieve and edit configuration files may expose sensitive information if the configurations contain secrets or credentials. Consider restricting access to these endpoints to administrative roles or sanitizing sensitive data before exposure.

Recommendations:

  • Restrict Access: Limit these endpoints to admin roles.
  • Sanitize Output: Exclude or mask sensitive information in the configuration data.
  • Secure Transmission: Ensure that the API uses HTTPS to protect data in transit.
  • Audit Logging: Implement logging for access and modification attempts for auditing purposes.

Implement logging for configuration access and modifications

To enhance security and traceability, add logging to record when configurations are listed, retrieved, or modified. This aids in monitoring usage and detecting unauthorized activities.

For example:

private static final Logger logger = LoggerFactory.getLogger(WebConfigController.class);

private void editConfig(Context ctx) {
    String user = ctx.sessionAttribute("userId");
    String configId = ctx.pathParam("configId");
    logger.info("User '{}' is modifying configuration '{}'", user, configId);
    // existing code...
}

private void retrieveConfigContent(Context ctx) {
    String user = ctx.sessionAttribute("userId");
    String configId = ctx.pathParam("configId");
    logger.info("User '{}' is retrieving configuration '{}'", user, configId);
    // existing code...
}

private void listConfig(Context ctx) {
    String user = ctx.sessionAttribute("userId");
    logger.info("User '{}' is listing configurations", user);
    // existing code...
}

Ensure sensitive information is not included in the logs.

Also applies to: 57-79, 81-93, 96-98

}

private void editConfig(Context ctx) throws IOException {
ConfigSaveRequest configSaveRequest = ctx.bodyAsClass(ConfigSaveRequest.class);
var test = new YamlConfiguration();
try {
test.loadFromString(configSaveRequest.content());
} catch (InvalidConfigurationException e) {
ctx.json(new StdResp(false, "Invalid configuration: " + e.getMessage(), null));
return;
}
switch (ctx.pathParam("configId")) {
case "config.yml" -> {
Files.writeString(Main.getMainConfigFile().toPath(), configSaveRequest.content(), StandardCharsets.UTF_8);
ctx.json(new StdResp(true, "Configuration saved", processConfigReloading()));
}
case "profile.yml" -> {
Files.writeString(Main.getProfileConfigFile().toPath(), configSaveRequest.content(), StandardCharsets.UTF_8);
ctx.json(new StdResp(true, "Configuration saved", processConfigReloading()));
}
default -> {
throw new IllegalArgumentException("Invalid configId");
}
}
}
Comment on lines +57 to +79
Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor to reduce code duplication in handling configuration files

The logic for handling configId in both editConfig and retrieveConfigContent methods is similar. Refactoring this logic into a helper method will improve maintainability.

Implement a helper method to get the configuration file path:

private Path getConfigFilePath(String configId) {
    return switch (configId) {
        case "config.yml" -> Main.getMainConfigFile().toPath();
        case "profile.yml" -> Main.getProfileConfigFile().toPath();
        default -> null;
    };
}

Refactor the methods:

private void editConfig(Context ctx) {
    ConfigSaveRequest configSaveRequest = ctx.bodyAsClass(ConfigSaveRequest.class);
    var test = new YamlConfiguration();
    try {
        test.loadFromString(configSaveRequest.content());
    } catch (InvalidConfigurationException e) {
        ctx.json(new StdResp(false, "Invalid configuration: " + e.getMessage(), null));
        return;
    }
    Path configFilePath = getConfigFilePath(ctx.pathParam("configId"));
    if (configFilePath == null) {
        ctx.status(400).json(new StdResp(false, "Invalid configId", null));
        return;
    }
    try {
        Files.writeString(configFilePath, configSaveRequest.content(), StandardCharsets.UTF_8);
        ctx.json(new StdResp(true, "Configuration saved", processConfigReloading()));
    } catch (IOException e) {
        ctx.json(new StdResp(false, "Error saving configuration: " + e.getMessage(), null));
    }
}

private void retrieveConfigContent(Context ctx) {
    Path configFilePath = getConfigFilePath(ctx.pathParam("configId"));
    if (configFilePath == null) {
        ctx.status(400).json(new StdResp(false, "Invalid configId", null));
        return;
    }
    try {
        String content = Files.readString(configFilePath, StandardCharsets.UTF_8);
        ctx.json(new StdResp(true, null, content));
    } catch (IOException e) {
        ctx.json(new StdResp(false, "Error reading configuration: " + e.getMessage(), null));
    }
}

Also applies to: 81-93


Handle exceptions and return appropriate error responses

In the editConfig method, exceptions like IOException during file writing are not caught, which may result in unhandled exceptions and internal server errors. It's important to handle these exceptions to provide meaningful feedback to the client.

Apply this diff to handle IOException and return proper error responses:

private void editConfig(Context ctx) {
    ConfigSaveRequest configSaveRequest = ctx.bodyAsClass(ConfigSaveRequest.class);
    var test = new YamlConfiguration();
    try {
        test.loadFromString(configSaveRequest.content());
    } catch (InvalidConfigurationException e) {
        ctx.json(new StdResp(false, "Invalid configuration: " + e.getMessage(), null));
        return;
    }
+   try {
        switch (ctx.pathParam("configId")) {
            case "config.yml" -> {
                Files.writeString(Main.getMainConfigFile().toPath(), configSaveRequest.content(), StandardCharsets.UTF_8);
                ctx.json(new StdResp(true, "Configuration saved", processConfigReloading()));
            }
            case "profile.yml" -> {
                Files.writeString(Main.getProfileConfigFile().toPath(), configSaveRequest.content(), StandardCharsets.UTF_8);
                ctx.json(new StdResp(true, "Configuration saved", processConfigReloading()));
            }
            default -> {
-               throw new IllegalArgumentException("Invalid configId");
+               ctx.status(400).json(new StdResp(false, "Invalid configId", null));
                return;
            }
        }
+   } catch (IOException e) {
+       ctx.json(new StdResp(false, "Error saving configuration: " + e.getMessage(), null));
+   }
}
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 void editConfig(Context ctx) throws IOException {
ConfigSaveRequest configSaveRequest = ctx.bodyAsClass(ConfigSaveRequest.class);
var test = new YamlConfiguration();
try {
test.loadFromString(configSaveRequest.content());
} catch (InvalidConfigurationException e) {
ctx.json(new StdResp(false, "Invalid configuration: " + e.getMessage(), null));
return;
}
switch (ctx.pathParam("configId")) {
case "config.yml" -> {
Files.writeString(Main.getMainConfigFile().toPath(), configSaveRequest.content(), StandardCharsets.UTF_8);
ctx.json(new StdResp(true, "Configuration saved", processConfigReloading()));
}
case "profile.yml" -> {
Files.writeString(Main.getProfileConfigFile().toPath(), configSaveRequest.content(), StandardCharsets.UTF_8);
ctx.json(new StdResp(true, "Configuration saved", processConfigReloading()));
}
default -> {
throw new IllegalArgumentException("Invalid configId");
}
}
}
private void editConfig(Context ctx) {
ConfigSaveRequest configSaveRequest = ctx.bodyAsClass(ConfigSaveRequest.class);
var test = new YamlConfiguration();
try {
test.loadFromString(configSaveRequest.content());
} catch (InvalidConfigurationException e) {
ctx.json(new StdResp(false, "Invalid configuration: " + e.getMessage(), null));
return;
}
try {
switch (ctx.pathParam("configId")) {
case "config.yml" -> {
Files.writeString(Main.getMainConfigFile().toPath(), configSaveRequest.content(), StandardCharsets.UTF_8);
ctx.json(new StdResp(true, "Configuration saved", processConfigReloading()));
}
case "profile.yml" -> {
Files.writeString(Main.getProfileConfigFile().toPath(), configSaveRequest.content(), StandardCharsets.UTF_8);
ctx.json(new StdResp(true, "Configuration saved", processConfigReloading()));
}
default -> {
ctx.status(400).json(new StdResp(false, "Invalid configId", null));
return;
}
}
} catch (IOException e) {
ctx.json(new StdResp(false, "Error saving configuration: " + e.getMessage(), null));
}
}


private void retrieveConfigContent(Context ctx) throws IOException {
switch (ctx.pathParam("configId")) {
case "config.yml" -> {
ctx.json(new StdResp(true, null, Files.readString(Main.getMainConfigFile().toPath(), StandardCharsets.UTF_8)));
}
case "profile.yml" -> {
ctx.json(new StdResp(true, null, Files.readString(Main.getProfileConfigFile().toPath(), StandardCharsets.UTF_8)));
}
default -> {
throw new IllegalArgumentException("Invalid configId");
}
}
}
Ghost-chu marked this conversation as resolved.
Show resolved Hide resolved


private void listConfig(Context ctx) {
ctx.json(new StdResp(true, null, new String[]{"config.yml", "profile.yml"}));
}

@Override
public void onDisable() {

}

private List<ConfigReloadEntryContainer> processConfigReloading() {
List<ConfigReloadEntryContainer> list = new ArrayList<>();
for (Map.Entry<ReloadableContainer, ReloadResult> entry : Main.reloadConfig().entrySet()) {
String name = "???";
var ref = entry.getKey().getReloadable();
if (ref != null) {
var obj = ref.get();
if (obj != null) {
name = obj.getClass().getName();
}
} else {
if (entry.getKey().getReloadableMethod() != null) {
name = entry.getKey().getReloadableMethod().getName();
}
}
list.add(new ConfigReloadEntryContainer(
name,
entry.getValue().getStatus().name(),
entry.getValue().getReason(),
entry.getValue().getException() == null ? null : entry.getValue().getException().getMessage()
));
}
return list;
}

record ConfigReloadEntryContainer(
String name,
String result,
String reason,
String errorMsg
) {

}

record ConfigSaveRequest(String content) {

}
}
5 changes: 5 additions & 0 deletions webui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,14 @@
},
"dependencies": {
"@arco-design/web-vue": "^2.56.2",
"@codemirror/lang-json": "^6.0.1",
"@codemirror/lang-yaml": "github:codemirror/lang-yaml",
"@dzangolab/flag-icon-css": "^3.4.5",
"@octokit/core": "^6.1.2",
"@octokit/request-error": "^6.1.4",
"@vueuse/core": "^11.1.0",
"codemirror": "^5",
"codemirror-editor-vue3": "^2.8.0",
"compare-versions": "^6.1.1",
"copy-to-clipboard": "^3.3.3",
"dayjs": "^1.11.13",
Expand All @@ -41,6 +45,7 @@
"@eslint/js": "^9.10.0",
"@rushstack/eslint-patch": "^1.10.4",
"@tsconfig/node20": "^20.1.4",
"@types/codemirror": "^5.60.15",
"@types/eslint__js": "^8.42.3",
"@types/lodash": "^4.17.7",
"@types/mockjs": "^1.0.10",
Expand Down
Loading
Loading