From ad1a699d388dbc40fe00c5026abbdacee60a422d Mon Sep 17 00:00:00 2001
From: Minecon724 <git@m724.eu>
Date: Sun, 2 Feb 2025 14:57:00 +0100
Subject: [PATCH] feat(auth): improve error handling and key generation logic

Added new error handling for key assignment failures, including user-facing messaging. Enhanced secure key generation with fixed length and randomness via `SecureRandom`. Removed redundant TODO comments and replaced placeholder exception handling with actionable implementations.
---
 .../m724/tweaks/module/auth/AuthListener.java |  6 ++++
 .../m724/tweaks/module/auth/AuthStorage.java  | 30 ++++++++-----------
 src/main/resources/strings.properties         |  1 +
 3 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/src/main/java/eu/m724/tweaks/module/auth/AuthListener.java b/src/main/java/eu/m724/tweaks/module/auth/AuthListener.java
index bc4634c..b22b677 100644
--- a/src/main/java/eu/m724/tweaks/module/auth/AuthListener.java
+++ b/src/main/java/eu/m724/tweaks/module/auth/AuthListener.java
@@ -6,6 +6,7 @@
 
 package eu.m724.tweaks.module.auth;
 
+import eu.m724.tweaks.DebugLogger;
 import eu.m724.tweaks.Language;
 import eu.m724.tweaks.config.TweaksConfig;
 import org.bukkit.entity.Player;
@@ -14,6 +15,7 @@ import org.bukkit.event.Listener;
 import org.bukkit.event.player.PlayerLoginEvent;
 
 import java.io.FileNotFoundException;
+import java.io.IOException;
 
 public class AuthListener implements Listener {
     private final AuthStorage authStorage;
@@ -40,6 +42,10 @@ public class AuthListener implements Listener {
                 allowed = true; // key just assigned
             } catch (FileNotFoundException | AuthStorage.AlreadyClaimedException | AuthStorage.InvalidKeyException e) {
                 allowed = !force; // If forced all players must have a key
+            } catch (IOException e) {
+                DebugLogger.severe("Error assigning key to player. " + e.getMessage());
+                event.disallow(PlayerLoginEvent.Result.KICK_OTHER, Language.getString("authKickError"));
+                allowed = true; // to skip the below checks
             }
         }
 
diff --git a/src/main/java/eu/m724/tweaks/module/auth/AuthStorage.java b/src/main/java/eu/m724/tweaks/module/auth/AuthStorage.java
index 7eb9c64..6692723 100644
--- a/src/main/java/eu/m724/tweaks/module/auth/AuthStorage.java
+++ b/src/main/java/eu/m724/tweaks/module/auth/AuthStorage.java
@@ -11,10 +11,15 @@ import org.bukkit.plugin.Plugin;
 import java.io.*;
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
+import java.security.SecureRandom;
 import java.util.Random;
 import java.util.UUID;
 
 public class AuthStorage {
+    private static final char[] KEY_CHARS = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789".toCharArray();
+    private static final int KEY_LENGTH = 10;
+    private static final SecureRandom RANDOM = new SecureRandom();
+
     private final File playersDirectory;
     private final File keysDirectory;
 
@@ -71,7 +76,7 @@ public class AuthStorage {
             byte[] bytes = is.readNBytes(50);
             return new String(bytes, StandardCharsets.UTF_8);
         } catch (IOException e) {
-            throw new RuntimeException(e); // TODO
+            throw new RuntimeException(e);
         }
 
     }
@@ -121,7 +126,7 @@ public class AuthStorage {
      * @throws FileNotFoundException if no such key
      * @throws AlreadyClaimedException if key is claimed or user owns another key
      */
-    void assignOwner(String key, UUID uuid) throws FileNotFoundException, AlreadyClaimedException {
+    void assignOwner(String key, UUID uuid) throws IOException, FileNotFoundException, AlreadyClaimedException {
         if (isInvalid(key)) throw new InvalidKeyException();
 
         if (getUserOfKey(key) != null) throw new AlreadyClaimedException();
@@ -136,34 +141,25 @@ public class AuthStorage {
 
         try (FileOutputStream os = new FileOutputStream(file)) {
             os.write(byteBuffer.array());
-        } catch (IOException e) {
-            throw new RuntimeException(e); // TODO
         }
 
         File file2 = new File(playersDirectory, uuid.toString());
 
         try (FileOutputStream os = new FileOutputStream(file2)) {
             os.write(key.getBytes(StandardCharsets.UTF_8));
-        } catch (IOException e) {
-            throw new RuntimeException(e); // TODO
         }
     }
 
-    // TODO improve
     String generateKey() {
-        char[] chars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789".toCharArray();
-        Random random = new Random();
-        int length = random.nextInt(8, 10);
+        StringBuilder builder = new StringBuilder();
 
-        StringBuilder key = new StringBuilder();
-
-        for (int i=0; i<length; i++) {
-            key.append(chars[random.nextInt(chars.length)]);
+        for (int i=0; i<KEY_LENGTH; i++) {
+            builder.append(KEY_CHARS[RANDOM.nextInt(KEY_CHARS.length)]);
         }
 
-        return key.toString();
+        return builder.toString();
     }
 
-    static class InvalidKeyException extends RuntimeException {}
-    static class AlreadyClaimedException extends Exception {}
+    static class InvalidKeyException extends RuntimeException { }
+    static class AlreadyClaimedException extends Exception { }
 }
diff --git a/src/main/resources/strings.properties b/src/main/resources/strings.properties
index 9394be9..ccef38a 100644
--- a/src/main/resources/strings.properties
+++ b/src/main/resources/strings.properties
@@ -29,6 +29,7 @@ chatAlreadyHere = You're already in this room
 authKickWrongKey = You're connecting to the wrong server address. You must connect to the one you're registered to.
 # If force is enabled and player is not registered. Changing this reveals you're using this plugin
 authKickUnregistered = You are not whitelisted on this server!
+authKickError = An error occured. Please try again. If this persists, contact an administrator.
 
 redstoneGatewayItem = Redstone gateway