CodeRabbit: feat(premium): cryptographic session verification for premium bypass#14
CodeRabbit: feat(premium): cryptographic session verification for premium bypass#14TuxCoding wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughThis PR adds a "Premium bypass" feature: Mojang-account verification lets enrolled players skip password authentication. It introduces premium enrollment commands (/premium, /freemium, /authme premium|freemium), persistence of a premium UUID column, proxy synchronization (Bungee/Velocity) and proxy-side login redirect config, PacketEvents-based cryptographic verification (RSA/AES) for direct offline-mode servers, pending-premium caching, and localized messages and docs. Several PacketEvents, platform adapter, proxy bridge, and join/login flows were extended to support verification, pending state, and proxy-aware auto-login. Sequence Diagram(s)sequenceDiagram
actor Client
participant Server as "Bukkit Server"
participant PacketEvents as "PacketEvents Listener"
participant Verifier as "PremiumLoginVerifier"
participant Mojang as "Mojang API"
participant Database as "DataSource"
participant Auth as "Authentication Flow"
Client->>Server: LOGIN_START
PacketEvents->>Database: Query premium flag / pending
Database-->>PacketEvents: premium? / pending?
alt premium or pending
PacketEvents->>Verifier: startVerification(connectionKey, username, playerUUID)
Verifier-->>PacketEvents: verifyToken
PacketEvents->>Client: ENCRYPTION_REQUEST (server public key + token)
Client->>Server: ENCRYPTION_RESPONSE (encrypted sharedSecret + token)
PacketEvents->>Verifier: decrypt token, validate
PacketEvents->>PacketEvents: install AES/CFB8 ciphers
Verifier->>Mojang: hasJoined(username, serverHash)
Mojang-->>Verifier: mojangUuid (optional)
alt Mojang verification succeeds
Verifier->>Database: (optional) record/store verified UUID or return verified UUID
PacketEvents->>Auth: resume login as verified (auto-login)
else
PacketEvents->>Auth: resume normal login (no auto-login)
end
else
PacketEvents->>Auth: resume normal login
end
sequenceDiagram
actor Player
participant Command as "Command Handler"
participant Service as "PremiumService"
participant DataSource as "DataSource"
participant Mojang as "MojangApiService"
participant Cache as "PlayerCache"
participant Proxy as "Bungee/Velocity Sender"
Player->>Command: /premium
Command->>Service: enablePremium(player)
Service->>Cache: get PlayerAuth
Cache-->>Service: PlayerAuth
alt player UUID is v4 (Mojang)
Service->>Service: storePremiumUuid (sync)
else
Service->>Mojang: fetchUuidByName(playerName)
Mojang-->>Service: mojangUuid (optional)
alt mojangUuid found
Service->>Service: add pending, notify proxy (premium.pending.set)
Service->>Player: send pending_kick message (sync kick scheduled)
else
Service-->>Player: send account_not_found
end
end
Service->>DataSource: updatePremiumUuid(auth)
DataSource-->>Service: success/failure
Service->>Proxy: sendPremiumSet/sendPremiumUnset as needed
sequenceDiagram
actor Client
participant Proxy as "Proxy (Bungee/Velocity)"
participant Backend as "AuthMe Backend"
participant BackendAuth as "AuthMe Auth Flow"
Client->>Proxy: Connect (premium username)
Proxy->>Proxy: check premiumUsernames / pending
alt username in proxy premium cache
Proxy->>Client: force online-mode (PreLogin)
end
Client->>Proxy: Login (Mojang verified)
Proxy->>Proxy: mark proxyVerifiedPremium
Proxy->>Backend: ServerConnectEvent to auth server
Backend->>Backend: allow connection if proxyVerifiedPremium && not pending
alt bypass allowed
BackendAuth->>BackendAuth: auto-login (skip dialog)
else
BackendAuth->>BackendAuth: normal auth flow
end
Backend->>Proxy: send premium.set/premium.list updates as needed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
authme-core/src/main/resources/messages/messages_eu.yml (1)
218-236:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPlease proofread the new premium translations.
Line 220 has a Basque typo (
gaitu→gaituta), and the new premium/admin strings could use a quick native-speaker pass before release.Suggested fix
- feature_disabled: '&cPremium modua ez dago gaitu zerbitzari honetan.' + feature_disabled: '&cPremium modua ez dago gaituta zerbitzari honetan.'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@authme-core/src/main/resources/messages/messages_eu.yml` around lines 218 - 236, Fix the Basque typo in the premium messages by changing the value of the premium.feature_disabled entry from using "gaitu" to "gaituta", and perform a native-speaker proofread of all keys under premium (feature_disabled, account_not_found, already_enabled, enable_success, not_enabled, disable_success, error) and premium.admin (not_registered, already_enabled, account_not_found, enable_success, not_enabled, disable_success, impostor_kicked, kick_reason) to ensure idiomatic phrasing and correct grammar before merging.
🧹 Nitpick comments (1)
authme-core/src/test/java/fr/xephi/authme/platform/AbstractSpigotPlatformAdapterTest.java (1)
124-130: ⚡ Quick winAdd assertions for premium-verification delegation path.
Lines 125–130 are no-ops in the tracking adapter, so new
registerPremiumVerification/unregisterPremiumVerificationbehavior is currently untested here. Add counters and assert calls in the lazy-init/reuse test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@authme-core/src/test/java/fr/xephi/authme/platform/AbstractSpigotPlatformAdapterTest.java` around lines 124 - 130, The test currently leaves the tracking adapter's registerPremiumVerification and unregisterPremiumVerification as no-ops, so add counters to the test adapter and assert the delegation in the lazy-init/reuse test: modify the anonymous TrackingPlatformAdapter used in AbstractSpigotPlatformAdapterTest to include atomic or integer counters for registerPremiumVerification and unregisterPremiumVerification, increment those counters inside the overridden registerPremiumVerification(DataSource, PremiumLoginVerifier) and unregisterPremiumVerification() methods, then update the lazy-init/reuse test to assert the expected counts after the operations that should trigger registration and unregistration (use the same test setup that checks other delegation counters so you reuse symbols like registerPremiumVerification and unregisterPremiumVerification for clarity).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@authme-bungee/src/main/java/fr/xephi/authme/bungee/BungeeProxyBridge.java`:
- Around line 183-189: The premium.list ingestion saves mixed-case names which
later fail lowercase lookups; update the loop in BungeeProxyBridge where
premiumUsernames is populated from parsedMessage.playerName() to normalize each
entry (e.g., toLowerCase()) before adding; ensure you call
name.trim().toLowerCase() (or equivalent normalization) when adding to
premiumUsernames so lookups using lowercase names match; keep the split and
empty checks but store normalized names consistently.
In
`@authme-core/src/main/java/fr/xephi/authme/command/executable/authme/SetFreemiumAdminCommand.java`:
- Around line 19-20: The executeCommand method in SetFreemiumAdminCommand
currently calls arguments.get(0) without checking size and can throw on empty
args; add a guard at the start of SetFreemiumAdminCommand.executeCommand to
check arguments.isEmpty() (or arguments.size() < 1) and if true send a helpful
message to the CommandSender (e.g., usage or error) and return early, otherwise
call premiumService.disablePremiumAdmin(sender, arguments.get(0)).
In
`@authme-core/src/main/java/fr/xephi/authme/service/bungeecord/BungeeReceiver.java`:
- Around line 98-104: The PROXY_STARTED handler in BungeeReceiver calls
dataSource.getPremiumUsernames() synchronously (hot path) which can block
startup; change this by moving the premium list lookup off the handler: either
have CacheDataSource implement caching for getPremiumUsernames() with a periodic
refresh, or load/update the premium list asynchronously on server start and keep
it in memory, then have the PROXY_STARTED branch read the cached list (or a safe
empty fallback) and call bungeeSender.sendPremiumList(player, premiumNames);
also add error handling/logging around retrieving the cached list so failures
don't block the handshake.
In
`@authme-core/src/main/java/fr/xephi/authme/service/bungeecord/BungeeSender.java`:
- Around line 142-155: sendPremiumNotification currently drops messages if no
carrier is online; change it to enqueue the payload when
bukkitService.getOnlinePlayers().stream().findFirst() is empty and flush the
queue when a carrier appears. Add a thread-safe queue field (e.g.,
ConcurrentLinkedQueue<byte[]> or a small Message POJO) to BungeeSender, push the
payload into it and log a debug/info when enqueued; keep the existing behavior
of scheduling sendAuthMePluginMessage via
bukkitService.scheduleSyncTaskFromOptionallyAsyncTask when a carrier is present.
Also add a listener or a scheduled flush method (invoked from a player-join
handler or a repeating task) that retrieves a carrier and drains the queue by
sending each queued payload with bukkitService.sendAuthMePluginMessage; ensure
thread-safety when draining the queue.
In `@authme-core/src/main/java/fr/xephi/authme/service/PremiumLoginVerifier.java`:
- Around line 155-175: The verified cache only evicts expired entries on reads
via getVerifiedUuid, causing stale entries to accumulate; fix by adding
proactive cleanup when mutating or periodically: update storeVerified (and/or
constructor) to purge expired entries from the verified map before inserting by
iterating verified.entrySet() and removing entries whose
VerifiedSession.verifiedAt() is older than VERIFIED_TTL_MS, or alternatively
schedule a background task (ScheduledExecutorService) that runs periodically to
remove entries based on VERIFIED_TTL_MS; reference symbols: verified,
storeVerified, getVerifiedUuid, VERIFIED_TTL_MS, VerifiedSession.
- Around line 130-142: The supplyAsync call in PremiumLoginVerifier is using the
JVM common pool while mojangApiService.hasJoined(...) performs blocking HTTP
I/O; change CompletableFuture.supplyAsync(...) to use a dedicated Executor
(e.g., premiumVerificationExecutor) by calling CompletableFuture.supplyAsync(()
-> { ... }, premiumVerificationExecutor); add that Executor as a
constructor-injected field on PremiumLoginVerifier (or obtain from a shared
scheduler), ensure it's sized for blocking verification tasks and properly
shutdown elsewhere, and keep the existing logic involving rsaDecrypt,
computeServerHash and pend.username() inside the task unchanged.
- Around line 73-77: The pending map entries created in startVerification (which
stores PendingVerification with startedAt) are never evicted; add time-based
eviction to prevent unbounded growth by removing stale PendingVerification
entries older than a defined timeout (e.g., 60s). Implement this by (a) running
a small scheduled cleanup task (ScheduledExecutorService) that iterates the
pending map and removes entries whose PendingVerification.startedAt is older
than now - TIMEOUT, and (b) defensive-check/removal in any lookup methods (e.g.,
verify/complete methods that read from pending) to drop and treat expired
entries as absent; reference the pending map, the PendingVerification.startedAt
field, and startVerification to locate where to add timeout logic and
scheduling.
In `@authme-core/src/main/java/fr/xephi/authme/service/PremiumService.java`:
- Around line 145-146: The auth/cache lookups use the raw playerName (e.g., in
the PlayerAuth retrieval via dataSource.getAuth and any cache lookups), but
stored keys are lowercased, so normalize the admin-supplied target before
lookups; update occurrences such as the call to dataSource.getAuth(playerName)
and any cache.get/contains calls (lines around PlayerAuth auth =
dataSource.getAuth(...), and the other spots noted) to use a consistently
lowercased form (e.g., playerName.toLowerCase(Locale.ROOT) or the project’s
canonical name-normalizer) and then proceed using that normalized variable for
auth and cache operations.
In `@authme-core/src/main/resources/messages/messages_ro.yml`:
- Around line 216-234: The translation key admin.spawn.first_not_defined was
accidentally nested under premium.admin as first_not_defined; move that entry
back to its proper namespace by removing premium.admin.first_not_defined and
adding admin.spawn.first_not_defined with the same message text ("&cPrimul spawn
a eșuat, te rugăm să încearcă să definești primul spawn.") so the locale
contains admin -> spawn -> first_not_defined instead of premium -> admin ->
first_not_defined; update only the YAML structure (no message text changes) and
keep other premium.admin keys unchanged.
In
`@authme-paper-common/src/main/java/fr/xephi/authme/listener/PaperDialogFlowListener.java`:
- Around line 319-324: The current early-return in PaperDialogFlowListener that
returns true when the player's UUID is still offline (non-v4) causes a fail-open
bypass; change the branch that currently does "return true" to return false
(i.e. do not skip the blocking pre-join dialog) when player.getUniqueId() is not
the resolved/proxy-forwarded Mojang UUID, and let
AsynchronousJoin.canBypassWithPremium() perform the definitive premium ownership
check later; update the method in PaperDialogFlowListener where the comment and
"return true" appear so unresolved UUIDs block until verified.
In
`@authme-paper-common/src/main/java/fr/xephi/authme/platform/AbstractPaperPlatformAdapter.java`:
- Around line 94-107: Replace the fragile reflection in
isPaperVelocityForwardingEnabled (in AbstractPaperPlatformAdapter) that queries
io.papermc.paper.configuration.GlobalConfiguration; instead implement a stable
detection strategy: remove reliance on that internal class and either (a) read
documented server config flags (online-mode and bungeecord from spigot.yml) to
infer Velocity forwarding, or (b) perform detection at player login by
inspecting the player's connection metadata/GameProfile properties for Velocity
forwarding signatures (preferred) and expose that result to callers, or (c) fall
back to explicit config option for users to set; update any callers of
isPaperVelocityForwardingEnabled to use the new mechanism and delete the
reflection-based code path.
In
`@authme-velocity/src/main/java/fr/xephi/authme/velocity/VelocityProxyBridge.java`:
- Around line 259-261: The current gate computes isPremiumJoin using only
proxyVerifiedPremium which allows proxy-verified players to bypass
authentication; change the logic in VelocityProxyBridge so isPremiumJoin
requires the player to be both proxyVerifiedPremium and present in
premiumUsernames (i.e., check proxyVerifiedPremium.contains(normalizedName) &&
premiumUsernames.contains(normalizedName)), update the conditional that uses
isPremiumJoin (the block with
authenticationStore.isAuthenticated(normalizedName) and logger.debug) and apply
the same fix to the analogous check around lines 264-267 so premium.unset takes
effect immediately.
- Around line 220-225: The premium list CSV is being cached without
normalization causing mismatches with the normalized checks in onPreLogin;
update the block that populates premiumUsernames (where
parsedMessage.playerName() is split) to normalize each entry (trim and
toLowerCase using the same normalization logic used in onPreLogin) before adding
to premiumUsernames so stored names match the normalized comparisons (refer to
premiumUsernames, parsedMessage.playerName(), and onPreLogin for the
normalization convention).
In `@docs/premium.md`:
- Around line 34-37: Add language identifiers to the fenced code blocks in the
premium doc to silence MD040: update the block containing "/premium — opt in
(must be logged in with password first) /freemium — opt out (reverts to
password authentication)", the block containing "/authme premium <player> —
enrol a player /authme freemium <player> — remove a player from premium
bypass", and the block containing the "Client Server (AuthMe + PacketEvents)
Mojang ... AsynchronousJoin: getVerifiedUuid(name) == auth.getPremiumUuid() →
auto-login" example by changing their opening fences to include an appropriate
language tag (e.g., ```text or ```bash) so each fenced code block starts with a
language specifier.
In `@README.md`:
- Line 162: The README bullet for PacketEvents is misleadingly global; update
the line referencing "PacketEvents 2.x (optional plugin; required for inventory
protection, tab-complete blocking, and premium bypass)" to clarify that
PacketEvents is required only for direct-connection or offline-mode setups (not
for backend proxy deployments), and add a short parenthetical referencing the
"premium-bypass" section so readers know backend proxy deployments are exempt.
Ensure you modify the "PacketEvents" bullet text and, if present,
cross-reference the "premium-bypass" section to keep the docs consistent.
---
Outside diff comments:
In `@authme-core/src/main/resources/messages/messages_eu.yml`:
- Around line 218-236: Fix the Basque typo in the premium messages by changing
the value of the premium.feature_disabled entry from using "gaitu" to "gaituta",
and perform a native-speaker proofread of all keys under premium
(feature_disabled, account_not_found, already_enabled, enable_success,
not_enabled, disable_success, error) and premium.admin (not_registered,
already_enabled, account_not_found, enable_success, not_enabled,
disable_success, impostor_kicked, kick_reason) to ensure idiomatic phrasing and
correct grammar before merging.
---
Nitpick comments:
In
`@authme-core/src/test/java/fr/xephi/authme/platform/AbstractSpigotPlatformAdapterTest.java`:
- Around line 124-130: The test currently leaves the tracking adapter's
registerPremiumVerification and unregisterPremiumVerification as no-ops, so add
counters to the test adapter and assert the delegation in the lazy-init/reuse
test: modify the anonymous TrackingPlatformAdapter used in
AbstractSpigotPlatformAdapterTest to include atomic or integer counters for
registerPremiumVerification and unregisterPremiumVerification, increment those
counters inside the overridden registerPremiumVerification(DataSource,
PremiumLoginVerifier) and unregisterPremiumVerification() methods, then update
the lazy-init/reuse test to assert the expected counts after the operations that
should trigger registration and unregistration (use the same test setup that
checks other delegation counters so you reuse symbols like
registerPremiumVerification and unregisterPremiumVerification for clarity).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 734f0c0f-10ef-4590-a394-2473822f4c95
📒 Files selected for processing (102)
README.mdauthme-bungee/src/main/java/fr/xephi/authme/bungee/AuthMeBungeePlugin.javaauthme-bungee/src/main/java/fr/xephi/authme/bungee/BungeeProxyBridge.javaauthme-bungee/src/main/java/fr/xephi/authme/bungee/BungeeProxyConfiguration.javaauthme-bungee/src/main/java/fr/xephi/authme/bungee/config/BungeeConfigProperties.javaauthme-bungee/src/test/java/fr/xephi/authme/bungee/BungeeProxyBridgeTest.javaauthme-bungee/src/test/java/fr/xephi/authme/bungee/BungeeReloadCommandTest.javaauthme-core/src/main/java/fr/xephi/authme/command/CommandInitializer.javaauthme-core/src/main/java/fr/xephi/authme/command/executable/authme/SetFreemiumAdminCommand.javaauthme-core/src/main/java/fr/xephi/authme/command/executable/authme/SetPremiumAdminCommand.javaauthme-core/src/main/java/fr/xephi/authme/command/executable/premium/FreemiumCommand.javaauthme-core/src/main/java/fr/xephi/authme/command/executable/premium/PremiumCommand.javaauthme-core/src/main/java/fr/xephi/authme/data/auth/PlayerAuth.javaauthme-core/src/main/java/fr/xephi/authme/datasource/AbstractSqlDataSource.javaauthme-core/src/main/java/fr/xephi/authme/datasource/CacheDataSource.javaauthme-core/src/main/java/fr/xephi/authme/datasource/Columns.javaauthme-core/src/main/java/fr/xephi/authme/datasource/DataSource.javaauthme-core/src/main/java/fr/xephi/authme/datasource/MySQL.javaauthme-core/src/main/java/fr/xephi/authme/datasource/PostgreSqlDataSource.javaauthme-core/src/main/java/fr/xephi/authme/datasource/SQLite.javaauthme-core/src/main/java/fr/xephi/authme/datasource/columnshandler/AuthMeColumns.javaauthme-core/src/main/java/fr/xephi/authme/listener/packetevents/AesCfb8Decoder.javaauthme-core/src/main/java/fr/xephi/authme/listener/packetevents/AesCfb8Encoder.javaauthme-core/src/main/java/fr/xephi/authme/listener/packetevents/PacketEventsListenerRegistry.javaauthme-core/src/main/java/fr/xephi/authme/listener/packetevents/PacketEventsService.javaauthme-core/src/main/java/fr/xephi/authme/listener/packetevents/PremiumVerificationPacketListener.javaauthme-core/src/main/java/fr/xephi/authme/message/MessageKey.javaauthme-core/src/main/java/fr/xephi/authme/message/updater/MessageUpdater.javaauthme-core/src/main/java/fr/xephi/authme/permission/AdminPermission.javaauthme-core/src/main/java/fr/xephi/authme/permission/PlayerPermission.javaauthme-core/src/main/java/fr/xephi/authme/platform/AbstractSpigotPlatformAdapter.javaauthme-core/src/main/java/fr/xephi/authme/platform/PacketInterceptionAdapter.javaauthme-core/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.javaauthme-core/src/main/java/fr/xephi/authme/service/MojangApiService.javaauthme-core/src/main/java/fr/xephi/authme/service/PremiumLoginVerifier.javaauthme-core/src/main/java/fr/xephi/authme/service/PremiumService.javaauthme-core/src/main/java/fr/xephi/authme/service/bungeecord/BungeeReceiver.javaauthme-core/src/main/java/fr/xephi/authme/service/bungeecord/BungeeSender.javaauthme-core/src/main/java/fr/xephi/authme/service/bungeecord/MessageType.javaauthme-core/src/main/java/fr/xephi/authme/settings/properties/AuthMeSettingsRetriever.javaauthme-core/src/main/java/fr/xephi/authme/settings/properties/DatabaseSettings.javaauthme-core/src/main/java/fr/xephi/authme/settings/properties/PremiumSettings.javaauthme-core/src/main/resources/messages/messages_bg.ymlauthme-core/src/main/resources/messages/messages_br.ymlauthme-core/src/main/resources/messages/messages_cz.ymlauthme-core/src/main/resources/messages/messages_de.ymlauthme-core/src/main/resources/messages/messages_en.ymlauthme-core/src/main/resources/messages/messages_eo.ymlauthme-core/src/main/resources/messages/messages_es.ymlauthme-core/src/main/resources/messages/messages_et.ymlauthme-core/src/main/resources/messages/messages_eu.ymlauthme-core/src/main/resources/messages/messages_fi.ymlauthme-core/src/main/resources/messages/messages_fr.ymlauthme-core/src/main/resources/messages/messages_gl.ymlauthme-core/src/main/resources/messages/messages_hu.ymlauthme-core/src/main/resources/messages/messages_id.ymlauthme-core/src/main/resources/messages/messages_it.ymlauthme-core/src/main/resources/messages/messages_ja.ymlauthme-core/src/main/resources/messages/messages_ko.ymlauthme-core/src/main/resources/messages/messages_lt.ymlauthme-core/src/main/resources/messages/messages_nl.ymlauthme-core/src/main/resources/messages/messages_pl.ymlauthme-core/src/main/resources/messages/messages_pt.ymlauthme-core/src/main/resources/messages/messages_ro.ymlauthme-core/src/main/resources/messages/messages_ru.ymlauthme-core/src/main/resources/messages/messages_si.ymlauthme-core/src/main/resources/messages/messages_sk.ymlauthme-core/src/main/resources/messages/messages_sr.ymlauthme-core/src/main/resources/messages/messages_tr.ymlauthme-core/src/main/resources/messages/messages_uk.ymlauthme-core/src/main/resources/messages/messages_vn.ymlauthme-core/src/main/resources/messages/messages_zhcn.ymlauthme-core/src/main/resources/messages/messages_zhhk.ymlauthme-core/src/main/resources/messages/messages_zhmc.ymlauthme-core/src/main/resources/messages/messages_zhtw.ymlauthme-core/src/main/resources/plugin.ymlauthme-core/src/test/java/fr/xephi/authme/command/CommandInitializerTest.javaauthme-core/src/test/java/fr/xephi/authme/platform/AbstractSpigotPlatformAdapterTest.javaauthme-core/src/test/java/fr/xephi/authme/process/join/AsynchronousJoinTest.javaauthme-core/src/test/java/fr/xephi/authme/service/bungeecord/BungeeReceiverTest.javaauthme-core/src/test/resources/fr/xephi/authme/datasource/sql-initialize.sqlauthme-core/src/test/resources/plugin.ymlauthme-folia/src/main/resources/plugin.ymlauthme-paper-common/pom.xmlauthme-paper-common/src/main/java/fr/xephi/authme/listener/PaperDialogFlowListener.javaauthme-paper-common/src/main/java/fr/xephi/authme/platform/AbstractPaperPlatformAdapter.javaauthme-paper-common/src/test/java/fr/xephi/authme/listener/PaperDialogFlowListenerTest.javaauthme-paper/src/main/resources/plugin.ymlauthme-spigot-1.21/src/main/resources/plugin.ymlauthme-spigot-legacy/src/main/resources/plugin.ymlauthme-velocity/src/main/java/fr/xephi/authme/velocity/AuthMeVelocityPlugin.javaauthme-velocity/src/main/java/fr/xephi/authme/velocity/VelocityProxyBridge.javaauthme-velocity/src/main/java/fr/xephi/authme/velocity/VelocityProxyConfiguration.javaauthme-velocity/src/main/java/fr/xephi/authme/velocity/config/VelocityConfigProperties.javaauthme-velocity/src/test/java/fr/xephi/authme/velocity/VelocityProxyBridgeTest.javadocs/commands.mddocs/config.mddocs/permission_nodes.mddocs/premium.mddocs/proxies/bungee/config.ymldocs/proxies/velocity/config.ymldocs/translations.md
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.42.1)
authme-core/src/main/java/fr/xephi/authme/service/PremiumLoginVerifier.java
[warning] 183-183: Detected SHA1 hash algorithm which is considered insecure. SHA1 is not collision resistant and is therefore not suitable as a cryptographic signature. Instead, use PBKDF2 for password hashing or SHA256 or SHA512 for other hash function applications.
Context: MessageDigest.getInstance("SHA-1")
Note: [CWE-328] Use of Weak Hash. [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
(use-of-sha1-java)
[warning] 177-177: Cipher in ECB mode is detected. ECB mode produces the same output for the same input each time which allows an attacker to intercept and replay the data. Further, ECB mode does not provide any integrity checking. See https://find-sec-bugs.github.io/bugs.htm#CIPHER_INTEGRITY.
Context: Cipher cipher = Cipher.getInstance("RSA/ECB/PKCS1Padding");
Note: [CWE-327] Use of a Broken or Risky Cryptographic Algorithm. [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
(ecb-cipher-java)
🪛 LanguageTool
docs/permission_nodes.md
[style] ~31-~31: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...mand to set the first AuthMe spawn. - authme.admin.setfreemium – Administrator com...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~32-~32: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... disable premium mode for a player. - authme.admin.setpremium – Administrator comm...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~67-~67: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ssion to see the own email address. - authme.player.freemium – Permission to disab...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~68-~68: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...Permission to disable premium mode. - authme.player.login – Command permission to ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~69-~69: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...in** – Command permission to login. - authme.player.logout – Command permission to...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~69-~69: Ensure spelling is correct
Context: ...player.logout** – Command permission to logout. - authme.player.premium – Permissi...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~70-~70: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...t** – Command permission to logout. - authme.player.premium – Permission to enable...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.22.1)
docs/premium.md
[warning] 34-34: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 42-42: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (56)
authme-core/src/main/resources/messages/messages_zhhk.yml (1)
221-238: Looks good.The new premium/admin strings fit the existing locale structure and cover the feature’s player/admin flows cleanly.
authme-core/src/main/resources/messages/messages_sr.yml (1)
218-235: Looks good.The new premium/admin translations are consistent with the existing message grouping and cover the added command paths.
authme-core/src/main/resources/messages/messages_tr.yml (1)
216-233: Looks good.The premium/admin strings are wired into the locale file consistently and match the new feature surface.
authme-core/src/main/java/fr/xephi/authme/settings/properties/DatabaseSettings.java (1)
141-144: Looks good.The new
MYSQL_COL_PREMIUM_UUIDsetting follows the existing database property pattern and cleanly exposes the premium UUID column name.authme-core/src/main/java/fr/xephi/authme/datasource/Columns.java (1)
34-34: Looks good.
PREMIUM_UUIDis exposed and populated consistently with the rest of the configurable column mappings.Also applies to: 58-58
authme-core/src/main/java/fr/xephi/authme/permission/PlayerPermission.java (1)
86-96: Looks good.The new premium permission nodes and updated enum terminator are consistent with the existing permission model.
authme-core/src/main/resources/messages/messages_en.yml (1)
216-233: Looks good.The premium/admin message keys are added in the expected place and line up with the new command flow.
authme-paper-common/pom.xml (1)
64-68: Dependency is correctly managed. Thech.jalu:datasourcecolumnsdependency is defined in the parent POM's dependencyManagement section (pom.xml:534-535), so omitting the version here is correct.authme-bungee/src/main/java/fr/xephi/authme/bungee/config/BungeeConfigProperties.java (1)
55-60: Looks good.The new
LOGIN_SERVERproperty fits the existing ConfigMe pattern and keeps the redirect target configurable without affecting the rest of the proxy settings.authme-core/src/main/resources/messages/messages_de.yml (1)
218-235: Looks good.The new
premiumblock is wired in cleanly and covers the added premium/admin message keys without disturbing the existing translations.authme-core/src/main/resources/messages/messages_fi.yml (1)
218-235: Looks good.The Finnish
premiumsection is added cleanly and aligns with the new message keys introduced by the premium flow.authme-core/src/main/resources/messages/messages_vn.yml (1)
218-235: Looks good.The new Vietnamese
premiumblock covers the added user/admin messages and fits the existing localization structure.authme-core/src/main/resources/messages/messages_eo.yml (1)
218-235: Looks good.The Esperanto
premiumblock is added in the right place and covers the new premium/admin keys cleanly.authme-core/src/main/resources/messages/messages_lt.yml (1)
218-235: Looks good.The Lithuanian
premiumblock lines up with the new namespace and introduces the new premium/admin messages cleanly.authme-bungee/src/test/java/fr/xephi/authme/bungee/BungeeProxyBridgeTest.java (2)
147-159: Looks good.The logout redirect test now matches the updated
BungeeProxyConfigurationconstructor signature.
441-444: Looks good.The shared test helper now supplies the new
loginServerargument, so the rest of the test cases stay aligned with the configuration API.authme-bungee/src/main/java/fr/xephi/authme/bungee/BungeeProxyConfiguration.java (1)
24-60: All constructor invocations have been correctly updated with the newloginServerargument in position 11, immediately precedingsharedSecretin position 12. No issues found.authme-bungee/src/test/java/fr/xephi/authme/bungee/BungeeReloadCommandTest.java (1)
29-38: Constructor call matches the updated config shape.The test still exercises the same reload path, and the extra argument lines up with the new
BungeeProxyConfigurationsignature.authme-core/src/main/java/fr/xephi/authme/service/bungeecord/MessageType.java (1)
10-13: Premium message IDs are wired in cleanly.The new enum constants follow the existing pattern, and
fromId()still works the same way.authme-core/src/main/java/fr/xephi/authme/permission/AdminPermission.java (1)
141-151: New admin permission nodes look consistent.The premium/freemium entries follow the existing enum shape and naming scheme.
authme-core/src/main/java/fr/xephi/authme/data/auth/PlayerAuth.java (1)
47-48: Premium UUID plumbing is consistent.The new field, accessor/mutator,
isPremium()helper, and builder assignment all line up cleanly.Also applies to: 187-197, 250-277, 390-393
authme-core/src/main/java/fr/xephi/authme/datasource/columnshandler/AuthMeColumns.java (1)
56-59: Premium UUID column mapping looks correct.The new column definition matches
PlayerAuth#getPremiumUuid()and stays optional.authme-core/src/main/java/fr/xephi/authme/datasource/MySQL.java (1)
297-300: MySQL schema and row mapping are in sync.The new
premium_uuidcolumn creation and result-set hydration line up with thePlayerAuthmodel update.Also applies to: 498-519
authme-core/src/main/java/fr/xephi/authme/datasource/PostgreSqlDataSource.java (1)
259-262: PostgreSQL schema and row mapping are aligned.The new
premium_uuidcolumn setup andpremiumUuid(...)builder wiring match the new auth model field.Also applies to: 460-480
authme-core/src/main/java/fr/xephi/authme/datasource/SQLite.java (1)
201-220: No issue found; the migration path properly preserves thepremium_uuidcolumn.
reload()invokessetup()before data migration (line 97 of SqLiteMigrater), ensuring the new table is created with all current columns includingPREMIUM_UUID. The subsequent INSERT statement copies legacy columns only because migrated databases predate this column. ThePREMIUM_UUIDcolumn structure is preserved; rows simply contain NULL values as expected for newly added columns on old data.authme-core/src/main/java/fr/xephi/authme/service/bungeecord/BungeeReceiver.java (1)
42-50: Constructor change is correctly wired everywhere.All
BungeeReceiverinstantiations (2 found in test files) have been updated to pass theDataSourceparameter. No compilation issues.authme-core/src/main/resources/messages/messages_ru.yml (1)
213-230: LGTM — the new premium localization block fits the existing message structure.The added
premiumandpremium.adminkeys look complete and consistent with the surrounding locale file.authme-core/src/main/resources/messages/messages_pl.yml (1)
218-235: LGTM — the premium messages are wired in cleanly.The new section is structurally sound and the added keys cover the expected user/admin premium flows.
authme-core/src/main/resources/messages/messages_hu.yml (1)
218-235: LGTM — no issues in the new premium block.The locale additions are consistent with the surrounding YAML and appear complete for the premium/admin messages.
authme-core/src/main/resources/messages/messages_gl.yml (1)
218-235: LGTM — the new premium localization entries look good.The added keys line up with the intended premium feature flow and don’t introduce any YAML structure problems.
authme-core/src/main/resources/messages/messages_zhcn.yml (1)
218-235: LGTM — the premium message additions are consistent.The new top-level
premiumsection and nested admin keys integrate cleanly with the existing message tree.authme-core/src/main/resources/messages/messages_uk.yml (1)
218-235: LGTM — the premium localization block looks complete.The new entries match the surrounding file’s structure and cover the expected premium/admin states.
authme-core/src/main/resources/messages/messages_zhtw.yml (1)
220-237: LGTM — the added premium section is well-formed.The new keys appear consistent with the locale file’s conventions and should support the premium flows cleanly.
authme-core/src/main/resources/messages/messages_bg.yml (1)
217-234: LGTM — no concerns with the new premium messages.The section is structurally correct and the added strings cover the player/admin premium paths as expected.
authme-core/src/main/resources/messages/messages_it.yml (1)
219-237: Premium message block looks consistent and complete.Key coverage and YAML structure for
premium/premium.adminlook good.authme-core/src/main/resources/messages/messages_nl.yml (1)
218-236: Dutch premium translations are well-structured.The new
premiumnamespace appears correctly formed and aligned with expected key patterns.authme-core/src/main/resources/messages/messages_si.yml (1)
218-236: Premium keys are correctly added for this locale.Structure and placeholder usage are consistent with the rest of the file.
authme-core/src/main/resources/messages/messages_ja.yml (1)
217-234: Japanese premium message additions look good.The new keys are complete and consistently structured.
authme-core/src/main/resources/messages/messages_pt.yml (1)
218-236: Portuguese premium translations are clean and complete.The added keyset is well-formed and consistent with the message schema.
authme-core/src/main/java/fr/xephi/authme/service/MojangApiService.java (1)
42-119: Solid baseline for Mojang API integration.Error handling, timeout configuration, and safe UUID parsing paths are implemented cleanly.
docs/proxies/bungee/config.yml (1)
39-45: Looks good.The new
loginServerentry and comments match the intended redirect behavior and fit the existing generated config structure.authme-core/src/main/resources/messages/messages_id.yml (1)
218-235: Looks good.The new premium message keys and admin subkeys are consistent with the feature’s command and service flow.
docs/translations.md (1)
2-2: Looks consistent.The refreshed translation percentages and generation timestamps appear to be an expected auto-generated update.
Also applies to: 15-15, 19-21, 31-31, 34-34, 41-41, 47-47
authme-core/src/main/resources/messages/messages_cz.yml (1)
218-235: Looks good.The premium message additions are aligned with the new command/service paths and read naturally for the Czech locale.
authme-core/src/main/resources/messages/messages_es.yml (1)
219-236: Looks good.The new premium translations cover the expected user/admin flows and fit the existing message structure.
authme-velocity/src/main/java/fr/xephi/authme/velocity/config/VelocityConfigProperties.java (1)
55-60: Looks good.The new
LOGIN_SERVERproperty cleanly exposes the redirect target and preserves the existing default-disabled behavior.docs/permission_nodes.md (1)
2-2: Looks good.The new premium permission nodes line up with the added commands and are reflected correctly in the generated docs.
Also applies to: 31-32, 67-70, 83-83
authme-core/src/main/java/fr/xephi/authme/command/executable/authme/SetPremiumAdminCommand.java (1)
19-20: Remove this comment.The command framework enforces argument validation before
executeCommandis invoked.CommandMapper.mapPartsToCommand()validates that the argument count matches the command's definition usinghasSuitableArgumentCount(). If the argument count is invalid, it returnsINCORRECT_ARGUMENTSstatus, andCommandHandlersends an error message without executing the command. SinceSetPremiumAdminCommandis registered with one mandatory argument,/authme setpremiumwithout a player name will be rejected by the framework before reaching this code. Thearguments.get(0)call is safe.authme-core/src/main/java/fr/xephi/authme/listener/packetevents/AesCfb8Encoder.java (1)
24-27:⚠️ Potential issue | 🟡 MinorGuard against null cipher output before writing
The
Cipher.update(byte[])method returns null for zero-length input (per Java SE documentation). If an empty buffer reaches this encoder, line 26 will attempt to write null bytes, causing a NullPointerException.Suggested fix
protected void encode(ChannelHandlerContext ctx, ByteBuf in, ByteBuf out) { byte[] input = new byte[in.readableBytes()]; in.readBytes(input); + byte[] encrypted = cipher.update(input); + if (encrypted != null) { + out.writeBytes(encrypted); + } }> Likely an incorrect or invalid review comment.authme-velocity/src/main/java/fr/xephi/authme/velocity/VelocityProxyConfiguration.java (1)
45-46: TheLOGIN_SERVERproperty has a default value of""andnormalizeServerName()safely handles empty strings—it simply returns the trimmed and lowercased input. Empty server names are also explicitly checked for at usage time (VelocityProxyBridge line 499), indicating they are expected and valid. No startup failure risk exists for missing or blank values.> Likely an incorrect or invalid review comment.authme-core/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java (1)
220-225: Good race-condition guard before limbo/dialog scheduling.The early and in-task
playerCache.isAuthenticated(...)checks are a solid fix for the proxy timing window and should prevent permanent limbo freezes.Also applies to: 253-256
authme-core/src/main/java/fr/xephi/authme/listener/packetevents/PacketEventsService.java (1)
56-60: Premium verification listener lifecycle is well handled.The gating and unregister paths are clean, and disabling/unloading correctly tears down premium verification hooks.
Also applies to: 103-111, 126-129
authme-paper-common/src/test/java/fr/xephi/authme/listener/PaperDialogFlowListenerTest.java (1)
297-459: Strong coverage for premium pre-join bypass logic.Nice additions here: verified/unverified paths, proxy UUID variants, and impostor mismatch behavior are all covered, which should protect the new bypass flow from regressions.
authme-core/src/test/resources/plugin.yml (1)
24-25: Command and permission wiring looks consistent.The new
premium/freemiumcommand and permission nodes are correctly reflected in command usage, top-level command entries, and wildcard permission children.Also applies to: 64-69, 94-95, 163-168, 269-320
authme-folia/src/main/resources/plugin.yml (1)
43-44: Folia plugin metadata updates look good.The premium/freemium command exposure and permission tree updates are coherent and aligned with the rest of the descriptor.
Also applies to: 83-88, 113-114, 182-187, 288-339
authme-core/src/main/java/fr/xephi/authme/listener/packetevents/PremiumVerificationPacketListener.java (1)
82-95: No thread-affinity issue here; the code is correct as-is.PacketEvents explicitly documents that
User.sendPacket()is thread-safe and can be safely called from arbitrary async threads without marshalling to the channel event loop. The underlying Netty operations handle thread-safety internally. The proposedrunOnEventLoop()wrapper pattern is unnecessary and adds unnecessary complexity.> Likely an incorrect or invalid review comment.
| private void sendPremiumNotification(MessageType type, String username) { | ||
| if (!isEnabled || !plugin.isEnabled()) { | ||
| return; | ||
| } | ||
| Optional<? extends Player> carrier = bukkitService.getOnlinePlayers().stream().findFirst(); | ||
| carrier.ifPresent(p -> { | ||
| ByteArrayDataOutput out = ByteStreams.newDataOutput(); | ||
| out.writeUTF(type.getId()); | ||
| out.writeUTF(username.toLowerCase(java.util.Locale.ROOT)); | ||
| byte[] payload = out.toByteArray(); | ||
| bukkitService.scheduleSyncTaskFromOptionallyAsyncTask(() -> | ||
| bukkitService.sendAuthMePluginMessage(p, payload)); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Don't drop premium updates when no carrier is online.
sendPremiumNotification() silently returns when the server is empty, so /premium and /freemium changes can be persisted locally but never reach the proxy. That leaves proxy state stale until some later full resync.
Queue the message until a carrier exists, or at least log/retry so the update is not lost.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@authme-core/src/main/java/fr/xephi/authme/service/bungeecord/BungeeSender.java`
around lines 142 - 155, sendPremiumNotification currently drops messages if no
carrier is online; change it to enqueue the payload when
bukkitService.getOnlinePlayers().stream().findFirst() is empty and flush the
queue when a carrier appears. Add a thread-safe queue field (e.g.,
ConcurrentLinkedQueue<byte[]> or a small Message POJO) to BungeeSender, push the
payload into it and log a debug/info when enqueued; keep the existing behavior
of scheduling sendAuthMePluginMessage via
bukkitService.scheduleSyncTaskFromOptionallyAsyncTask when a carrier is present.
Also add a listener or a scheduled flush method (invoked from a player-join
handler or a repeating task) that retrieves a carrier and drains the queue by
sending each queued payload with bukkitService.sendAuthMePluginMessage; ensure
thread-safety when draining the queue.
| private static boolean isPaperVelocityForwardingEnabled() { | ||
| try { | ||
| // io.papermc.paper.configuration.GlobalConfiguration is Paper 1.18+ | ||
| Class<?> cfg = Class.forName( | ||
| "io.papermc.paper.configuration.GlobalConfiguration", | ||
| false, AbstractPaperPlatformAdapter.class.getClassLoader()); | ||
| Object instance = cfg.getMethod("get").invoke(null); | ||
| Object proxies = cfg.getField("proxies").get(instance); | ||
| Object velocity = proxies.getClass().getField("velocity").get(proxies); | ||
| Object enabled = velocity.getClass().getField("enabled").get(velocity); | ||
| return Boolean.TRUE.equals(enabled); | ||
| } catch (Exception ignored) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For currently supported Paper versions, does io.papermc.paper.configuration.GlobalConfiguration reliably expose get().proxies.velocity.enabled as public reflective fields? If not, what stable API/config path is recommended to detect Velocity forwarding?
💡 Result:
No, io.papermc.paper.configuration.GlobalConfiguration does not reliably expose get.proxies.velocity.enabled as public reflective fields. While the class exists in io.papermc.paper.configuration and provides a static get method returning a singleton instance with nested public fields matching the paper-global.yml structure (including proxies.velocity.enabled), this is an internal Paper implementation detail using Configurate's ConfigurationPart system for automatic YAML serialization/deserialization. It is not part of the stable public Paper/Spigot/Bukkit API and should not be used by plugins, as it lacks versioning guarantees, may change between builds/versions, and is not documented in official JavaDocs as a stable API. The recommended stable way to detect Velocity forwarding in plugins is to check player connection metadata or forwarding headers provided by Velocity's modern forwarding protocol. When enabled, Velocity injects specific player info (UUID, IP, skin) via a custom login packet that Paper recognizes. Plugins can detect this by: 1. Checking if the server's online-mode is false (required for forwarding) and bungeecord is false in spigot.yml (to distinguish from BungeeCord). 2. Using ServerPlayer#getRemoteAddress or connection metadata, as Velocity sets the real client IP in the forwarded data. 3. Listening for login events and inspecting GameProfile properties, which contain forwarded data when Velocity is active. For runtime detection without config access, inspect if player connections have Velocity-specific forwarding signatures (e.g., extra data in Login packet). Official docs emphasize config.path proxies.velocity.enabled for server admins, but plugins should use protocol/runtime detection for compatibility.
Citations:
- 1: https://docs.papermc.io/paper/reference/global-configuration/
- 2: https://papermc-paper.mintlify.app/administration/paper-configuration
- 3: https://docs.papermc.io/velocity/player-information-forwarding/
- 4: https://papermc-paper.mintlify.app/concepts/configuration-system
- 5: https://papermc-paper.mintlify.app/api/paper-api
🏁 Script executed:
cd authme-paper-common && rg "isPaperVelocityForwardingEnabled" -A 5 -B 5Repository: TuxCoding/AuthMeReloaded
Length of output: 2501
🏁 Script executed:
cd authme-paper-common && cat -n src/main/java/fr/xephi/authme/platform/AbstractPaperPlatformAdapter.java | sed -n '90,115p'Repository: TuxCoding/AuthMeReloaded
Length of output: 1031
Velocity forwarding detection relies on an unstable internal API that will fail silently.
io.papermc.paper.configuration.GlobalConfiguration is not a stable public API and lacks versioning guarantees—it may change between builds/versions. Any reflection failure on line 106 returns false, which incorrectly marks Velocity as disabled and breaks logins in proxied setups. Instead, use documented approaches: (1) check online-mode: false and bungeecord: false in spigot.yml to confirm Velocity forwarding mode, (2) inspect player connection metadata at login time to detect forwarded connections, or (3) examine GameProfile properties for Velocity-specific forwarding signatures. These stable methods are compatible across versions and avoid silent failures.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@authme-paper-common/src/main/java/fr/xephi/authme/platform/AbstractPaperPlatformAdapter.java`
around lines 94 - 107, Replace the fragile reflection in
isPaperVelocityForwardingEnabled (in AbstractPaperPlatformAdapter) that queries
io.papermc.paper.configuration.GlobalConfiguration; instead implement a stable
detection strategy: remove reliance on that internal class and either (a) read
documented server config flags (online-mode and bungeecord from spigot.yml) to
infer Velocity forwarding, or (b) perform detection at player login by
inspecting the player's connection metadata/GameProfile properties for Velocity
forwarding signatures (preferred) and expose that result to callers, or (c) fall
back to explicit config option for users to set; update any callers of
isPaperVelocityForwardingEnabled to use the new mechanism and delete the
reflection-based code path.
| premiumUsernames.clear(); | ||
| if (!parsedMessage.playerName().isEmpty()) { | ||
| for (String name : parsedMessage.playerName().split(",")) { | ||
| if (!name.isEmpty()) { | ||
| premiumUsernames.add(name.trim()); | ||
| } |
There was a problem hiding this comment.
Normalize premium.list entries before caching.
onPreLogin checks normalized usernames, but premium.list currently stores raw CSV names. Mixed-case entries can silently fail to match.
Suggested fix
- for (String name : parsedMessage.playerName().split(",")) {
- if (!name.isEmpty()) {
- premiumUsernames.add(name.trim());
+ for (String name : parsedMessage.playerName().split(",")) {
+ String trimmed = name.trim();
+ if (!trimmed.isEmpty()) {
+ premiumUsernames.add(normalizeName(trimmed));
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@authme-velocity/src/main/java/fr/xephi/authme/velocity/VelocityProxyBridge.java`
around lines 220 - 225, The premium list CSV is being cached without
normalization causing mismatches with the normalized checks in onPreLogin;
update the block that populates premiumUsernames (where
parsedMessage.playerName() is split) to normalize each entry (trim and
toLowerCase using the same normalization logic used in onPreLogin) before adding
to premiumUsernames so stored names match the normalized comparisons (refer to
premiumUsernames, parsedMessage.playerName(), and onPreLogin for the
normalization convention).
| boolean isPremiumJoin = connectingToAuthServer && proxyVerifiedPremium.contains(normalizedName); | ||
| if (!authenticationStore.isAuthenticated(normalizedName) && !isPremiumJoin) { | ||
| logger.debug("Skipping auto-login for {} — not authenticated or proxy-verified premium", normalizedName); |
There was a problem hiding this comment.
Gate proxy premium bypass on premium enrollment, not only online-mode verification.
isPremiumJoin trusts proxyVerifiedPremium alone. That allows bypass as soon as a player is proxy-verified, and it makes premium.unset ineffective until disconnect. Require explicit membership in premiumUsernames in this gate.
Suggested fix
- boolean isPremiumJoin = connectingToAuthServer && proxyVerifiedPremium.contains(normalizedName);
+ boolean isPremiumJoin = connectingToAuthServer
+ && proxyVerifiedPremium.contains(normalizedName)
+ && premiumUsernames.contains(normalizedName);Also applies to: 264-267
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@authme-velocity/src/main/java/fr/xephi/authme/velocity/VelocityProxyBridge.java`
around lines 259 - 261, The current gate computes isPremiumJoin using only
proxyVerifiedPremium which allows proxy-verified players to bypass
authentication; change the logic in VelocityProxyBridge so isPremiumJoin
requires the player to be both proxyVerifiedPremium and present in
premiumUsernames (i.e., check proxyVerifiedPremium.contains(normalizedName) &&
premiumUsernames.contains(normalizedName)), update the conditional that uses
isPremiumJoin (the block with
authenticationStore.isAuthenticated(normalizedName) and logger.debug) and apply
the same fix to the analogous check around lines 264-267 so premium.unset takes
effect immediately.
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
authme-core/src/main/resources/messages/messages_et.yml (1)
1-238:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEncoding corruption (Mojibake) introduced across ~50+ changed Estonian message lines.
The file was edited in a Latin-1-mode environment: UTF-8 multibyte sequences were decoded as Latin-1 and then re-saved as UTF-8, producing double-encoded sequences. Examples from the changed lines:
Garbled (in file) Should be käsklusegakäsklusegamängijadmängijadTühistaTühistapäevpäevPremium-režiimPremium-režiimMängija, kes oli ühendatudMängija, kes oli ühendatudUnchanged lines (e.g.,
command_usage,match_error) are unaffected, confirming this is a PR-introduced regression.Fix: Revert all changes to existing pre-premium keys, then re-add only the
premiumblock (lines 217–238) using a UTF-8-aware editor. A quick Python one-liner can verify/restore corrupted characters:s.encode('latin-1').decode('utf-8').🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@authme-core/src/main/resources/messages/messages_et.yml` around lines 1 - 238, Many Estonian strings were double-encoded (mojibake) in keys like registration.register_request, password.match_error, dialog.button.cancel, time.day and others; revert all edits to the pre-existing keys (everything except the premium block) so their original UTF-8 text is restored, then re-add only the premium block (premium.feature_disabled ... premium.pending_fail) ensuring the file is saved in UTF-8; open/save with a UTF-8-aware editor or run a transliteration fix on the corrupted values (e.g. for a string s: s.encode('latin-1').decode('utf-8')) to convert garbled entries back to proper UTF-8 before committing.authme-core/src/main/resources/messages/messages_br.yml (1)
1-241:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame Mojibake encoding corruption as
messages_et.yml— affects Brazilian Portuguese messages in-game.Numerous changed lines have double-encoded UTF-8, making Portuguese-specific characters unreadable. Examples:
Garbled (in file) Should be TraduçãoTraduçãousuáriosusuáriosVocêVocênãonãoreconecte-se→reconecte-é-sevariantscorrect accented form Unchanged lines (e.g., Line 11, Line 29) are unaffected, confirming the corruption was introduced by this PR's edits. The premium block (Lines 222–241) is also affected.
Fix: Same as for
messages_et.yml— revert all changes to pre-existing keys and re-add only thepremiumblock using a UTF-8-aware editor.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@authme-core/src/main/resources/messages/messages_br.yml` around lines 1 - 241, This file contains double-encoded UTF-8 (mojibake) in many translation values; revert the edited translations back to their original pre-PR Portuguese strings for keys like registration.register_request, password.match_error, login.login_request, error.unregistered_user, on_join_validation.invalid_name_case, recovery.code.code_sent, dialog.* and other top-level sections, then re-add only the premium block (premium.* keys) using a UTF-8-aware editor and save the file as UTF-8 (no BOM) so accented characters (e.g., Tradução, usuários, Você, não, reconecte-se) are stored correctly; ensure no other keys are changed beyond restoring correct Portuguese text.
♻️ Duplicate comments (3)
docs/premium.md (1)
54-79:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winComplete the markdown linting fix: add language tag to this code block.
The past review flagged three code blocks (lines 34, 42, 54) for missing language tags. Only the first two were fixed — this block still triggers the MD040 warning.
📝 Proposed fix
-``` +```text Client Server (AuthMe + PacketEvents) Mojang🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/premium.md` around lines 54 - 79, Add a language tag to the fenced code block that begins with "Client Server (AuthMe + PacketEvents) Mojang" so the markdown linter (MD040) is satisfied; change the opening fence from ``` to ```text (or another appropriate language such as ```nohighlight) and leave the block contents unchanged.authme-core/src/main/java/fr/xephi/authme/service/PremiumService.java (1)
158-160:⚠️ Potential issue | 🟠 MajorNormalize the admin target before storage/cache lookups.
These paths still use raw
playerNamefordataSource.getAuth(...)andplayerCache.isAuthenticated(...). Mixed-case/authme premium|freemium <player>calls can miss the stored auth record or skip the cache refresh even though the account exists. Normalize once and use that key for persistence/cache operations.Also applies to: 228-249
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@authme-core/src/main/java/fr/xephi/authme/service/PremiumService.java` around lines 158 - 160, Normalize the admin target once at the start of the async task and use that normalized key for all persistence/cache calls: replace uses of the raw playerName inside the bukkitService.runTaskOptionallyAsync lambda (where dataSource.getAuth(playerName) and playerCache.isAuthenticated(playerName) are called) with a single normalized variable (e.g., normalizedPlayer or normalizedName) and use that same variable for any subsequent lookups/updates; apply the same change to the other block referenced (the logic around lines 228-249) so all dataSource and playerCache interactions consistently use the normalized key.authme-core/src/main/java/fr/xephi/authme/service/PremiumLoginVerifier.java (1)
89-103:⚠️ Potential issue | 🟠 MajorEnforce
PENDING_TTL_MSon reads, not just on new starts.
evictStalePendingEntries()only runs fromstartVerification(). An expired handshake can still passhasPending(...), return a username/UUID, and be accepted bycompleteVerification(...)long after the 30s window if no newer premium start happened. Expiry needs to be checked when reading/removing pending entries too.Also applies to: 133-138
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@authme-core/src/main/java/fr/xephi/authme/service/PremiumLoginVerifier.java` around lines 89 - 103, The pending-entry TTL is only enforced when starting verifications, so reads can return stale entries; update all read/remove methods (e.g., hasPending, getPendingUsername, getPendingPlayerUUID and the methods around lines 133-138 such as completeVerification/removePending) to check expiry before returning data. Implement this by either invoking evictStalePendingEntries() at the start of each read/remove method or by checking the PendingVerification's timestamp against PENDING_TTL_MS (and removing the map entry if expired) so expired handshakes are treated as absent and not returned or accepted.
🧹 Nitpick comments (4)
docs/premium.md (1)
183-187: ⚖️ Poor tradeoffClarify the account re-enrollment scenario after username change.
The phrase "depending on your account-linking configuration" is vague. Since AuthMe accounts are name-keyed and the premium bypass uses Mojang UUID, readers need concrete guidance:
- Will the old account (old name) still auto-login using the stored UUID?
- Must the player register a new account with the new name and re-enroll?
- Is there a migration path?
Consider expanding this answer to describe the actual behavior users will encounter.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/premium.md` around lines 183 - 187, Clarify that premium bypass is checked by Mojang UUID while AuthMe stores accounts by lowercase player name, so after a username change the premium check will still recognize the player (UUID), but AuthMe will not automatically map the new name to the old name-keyed account; instruct readers to either run /authme premium to re-link the new name to their premium status or use their server's AuthMe account-migration/rename feature (or a plugin setting that enables UUID-based account linking) to migrate the old name record to the new lowercase name so the player does not need to fully re-register.authme-core/src/main/resources/plugin.yml (1)
326-338: ⚡ Quick winConsider documenting the implications of
default: trueforauthme.player.premium.Both
authme.player.premiumandauthme.player.freemiumdefault totrue, meaning every player on the server can invoke/premium(and thereby attempt premium enrollment) without any explicit admin grant. This is consistent with all otherauthme.player.*defaults, and the actual bypass is protected by cryptographic Mojang session verification — so there is no direct auth-bypass risk.However, server admins may not realize that any player can self-enroll without admin intervention. A short note in the description or in the accompanying documentation (e.g., README / wiki) clarifying that admins must explicitly deny this node to restrict self-enrollment would prevent deployment surprises.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@authme-core/src/main/resources/plugin.yml` around lines 326 - 338, The permission node authme.player.premium currently has default: true (as does authme.player.freemium), which means any player can run /premium to attempt premium enrollment; update the authme.player.premium description (or README/wiki) to explicitly state that default:true allows all players to self-enroll and that server operators must explicitly deny this permission to prevent self-enrollment, and mention that enrollment is still gated by Mojang session verification for cryptographic protection.authme-core/src/test/java/fr/xephi/authme/service/bungeecord/BungeeReceiverTest.java (1)
99-127: ⚡ Quick winTest does not cover the premium verification flow.
The test stubs the necessary mocks for the basic PERFORM_LOGIN flow, but
pendingPremiumCachereturnsnullby default, so the premium verification branch (lines 164-176 inBungeeReceiver) is never exercised. Consider adding test cases that:
- Verify behavior when a pending premium UUID exists and matches the player's v4 UUID
- Verify behavior when the pending UUID exists but the player has a v3 UUID (offline mode)
- Verify behavior when the pending UUID doesn't match the player's UUID
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@authme-core/src/test/java/fr/xephi/authme/service/bungeecord/BungeeReceiverTest.java` around lines 99 - 127, Add tests that exercise the premium verification branch by stubbing a pending premium UUID and the player's UniqueId: create three new test cases (modeled on shouldQueueSessionAndForceLoginWhenPerformLoginReceivedForOnlinePlayer) where you set pendingPremiumCache to return a UUID for playerName, then set the mocked Player.getUniqueId() to (a) a matching v4 UUID, (b) a v3/offline-mode UUID, and (c) a non-matching UUID; in each test call receiver.onPluginMessageReceived(...) and verify the expected interactions with proxySessionManager.processProxySessionMessage, management.forceLoginFromProxy and bungeeSender.sendAuthMeBungeecordMessage and also verify pendingPremiumCache removal or premiumService invocation as appropriate to each branch so the premium verification logic in BungeeReceiver is covered.authme-core/src/test/java/fr/xephi/authme/process/join/AsynchronousJoinTest.java (1)
97-111: ⚡ Quick winLGTM with a suggestion for additional test coverage.
The
PremiumLoginVerifiermock is added andENABLE_PREMIUMis stubbed tofalseinsetUp(), which correctly disables the premium bypass path for existing non-premium tests. However, this means the newcanBypassWithPremium()logic inAsynchronousJoinis not exercised.Consider adding test cases with
ENABLE_PREMIUMset totrueto cover:
- Premium bypass with a v4 UUID (proxy-verified)
- Premium bypass with a v3 UUID requiring
PremiumLoginVerifierverification- Pending premium verification success/failure paths
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@authme-core/src/test/java/fr/xephi/authme/process/join/AsynchronousJoinTest.java` around lines 97 - 111, The test setup stubs PremiumSettings.ENABLE_PREMIUM to false so AsynchronousJoin.canBypassWithPremium() is never exercised; add new unit tests in AsynchronousJoinTest that set given(service.getProperty(PremiumSettings.ENABLE_PREMI)).willReturn(true) and exercise AsynchronousJoin.canBypassWithPremium() for (1) a v4 UUID path that should bypass immediately (proxy-verified), (2) a v3 UUID path that invokes the PremiumLoginVerifier mock (verify interactions with PremiumLoginVerifier) and (3) pending verification flows where the verifier returns success and failure to assert both acceptance and denial handling; update/instantiate the PremiumLoginVerifier mock behavior in each test and reuse existing helper methods from setUp() to schedule tasks as needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@authme-bungee/src/main/java/fr/xephi/authme/bungee/BungeeProxyBridge.java`:
- Around line 331-357: The onLogin and onPostLogin handlers currently mark every
online-mode or UUIDv4 player as proxy verified; change both to first check that
the normalizedName is present in premiumUsernames or pendingPremiumUsernames
before calling markProxyVerifiedPremium or adding to proxyVerifiedPremium. In
onLogin, after computing normalizedName call a membership check against
premiumUsernames || pendingPremiumUsernames and only then call
markProxyVerifiedPremium(normalizedName); in onPostLogin, compute
normalizedName, verify membership in premiumUsernames || pendingPremiumUsernames
and only then perform proxyVerifiedPremium.add(normalizedName) and log the
Mojang UUID message. Ensure you reference the existing symbols normalizedName,
markProxyVerifiedPremium, proxyVerifiedPremium, premiumUsernames,
pendingPremiumUsernames, onLogin and onPostLogin when making the change.
In
`@authme-core/src/main/java/fr/xephi/authme/command/executable/authme/ForceLoginCommand.java`:
- Around line 44-52: The pre-join branch calls
preJoinDialogService.approvePreJoinForceLogin(...) without checking the target's
CAN_LOGIN_BE_FORCED permission, so add a permission check before approving:
query the permission/authorization routine used elsewhere (e.g.,
PermissionNode.CAN_LOGIN_BE_FORCED or the same permission-check helper used for
online players) for playerName.toLowerCase(Locale.ROOT) and only call
preJoinDialogService.approvePreJoinForceLogin(...) if that check allows forced
logins; otherwise treat as denied and send the existing failure message
(MessageKey.FORCE_LOGIN_PLAYER_OFFLINE or a suitable denial message). Ensure you
reference the same permission constant/method as used for the online-player path
and keep the lowercasing key consistent.
In
`@authme-core/src/main/java/fr/xephi/authme/listener/packetevents/PremiumVerificationPacketListener.java`:
- Around line 123-132: The branch handling !encVerifyTokenOpt.isPresent()
resumes login without enabling channel encryption or decrypting encSharedSecret,
causing a connection desync after ENCRYPTION_RESPONSE; fix by replicating the
encryption setup used when a plain verify token is present: decrypt
encSharedSecret, install the AES encryption/decryption handlers on the channel
(same logic used elsewhere to enable encryption), and only then call
loginVerifier.cleanupPending(connectionKey), event.setCancelled(true) and
resumeLogin(user, username, clientVersion, playerUUID); alternatively, if
decryption cannot succeed here, abort the login instead of resuming to avoid
desync (use the same abort path as the verified-token failure handling).
- Around line 88-105: The async task started in
PremiumVerificationPacketListener must handle exceptions so cancelled
LOGIN_START doesn't leave connections hanging; wrap the
CompletableFuture.runAsync body in a try/catch, and on any thrown exception call
the same recovery path used in handleEncryptionResponse() — either
resumeLogin(user, username, clientVersion, playerUUID) or explicitly
fail/disconnect the user (using the existing disconnect/resume helpers) and log
the error; reference the pendingPremiumCache check, dataSource.getAuth(...),
loginVerifier.startVerification(...), and resumeLogin(...) when implementing the
catch to ensure every error path resumes or terminates the login flow.
In
`@authme-core/src/main/java/fr/xephi/authme/service/bungeecord/BungeeReceiver.java`:
- Around line 164-175: In BungeeReceiver, before calling
premiumService.finalizePendingPremium(player, playerId) (after
pendingPremiumCache.removePending(name)), verify that the player UUID returned
by player.getUniqueId() equals the pendingUuid removed from the cache; if they
match call finalizePendingPremium, otherwise log a warning similar to the
existing message and call bungeeSender.sendPremiumUnset(name) to discard the
pending state — i.e., replace the unconditional finalizePendingPremium call with
an equality check using playerId.equals(pendingUuid) and the same fallback
handling used for non-v4 UUIDs.
- Around line 109-118: The captured Player reference (carrier) used in the async
DB query may be stale if the player disconnects; modify the sync callback
scheduled from bukkitService.scheduleSyncTaskFromOptionallyAsyncTask so it
verifies the player is still online before calling bungeeSender.sendPremiumList
and logger.info (e.g., check carrier.isOnline() or re-resolve the player via its
UUID), and bail out if the player is no longer connected to avoid sending plugin
messages through a disconnected Player.
In
`@authme-core/src/main/java/fr/xephi/authme/service/bungeecord/BungeeSender.java`:
- Around line 144-150: sendPremiumList currently writes the entire
premiumUsernames as a single UTF string which can exceed BungeeCord's
32,767-byte plugin message limit; modify sendPremiumList to chunk the list and
send multiple plugin messages from the carrier. Build each chunk by iterating
premiumUsernames and accumulating comma-separated names until adding the next
name would exceed a safe byte-size threshold (e.g. 32KB minus overhead when
serialized as UTF), then for each chunk create a ByteArrayDataOutput
(ByteStreams.newDataOutput()), write MessageType.PREMIUM_LIST.getId() plus
simple chunk metadata (partIndex and totalParts or a continuation flag) and the
chunk payload, and send each chunk via the existing send logic; ensure
ByteArrayDataOutput/UTF sizing is respected so no single message exceeds the
proxy limit.
In `@authme-core/src/main/java/fr/xephi/authme/service/PendingPremiumCache.java`:
- Around line 62-72: The getPendingUuid method has a TOCTOU where
pending.remove(name.toLowerCase(Locale.ROOT)) can delete a newly added entry;
change the unconditional remove to the conditional remove(key, value) overload
so you only remove if the mapped value is the same expired PendingEntry you
observed. In practice, capture the key and the retrieved PendingEntry (from
pending.get(...)), check expiry as now done, and call pending.remove(key, entry)
instead of pending.remove(key) so concurrent addPending calls won't clobber a
fresh valid record; this relies on PendingEntry's record equals to match both
mojangUuid and expiresAt.
In `@authme-core/src/main/java/fr/xephi/authme/service/PreJoinDialogService.java`:
- Around line 63-116: approvePreJoinForceLogin can race with
clear/unregisterPreJoinFuture because it reads pendingPreJoinByName and
pendingPreJoinFutures then adds to pendingForceLogins without atomicity; make
the check-and-set operations atomic by introducing per-player synchronization
(e.g. a lock object stored in a ConcurrentHashMap keyed by UUID or by
synchronizing on a dedicated PlayerEntry object that consolidates
pendingPreJoinByName/pendingPreJoinFutures/pendingForceLogins state). Update
approvePreJoinForceLogin to acquire the per-player lock (or synchronize on the
PlayerEntry), re-check that uuid and future are present, add to
pendingForceLogins and only then complete the future, and release the lock;
modify clear/unregisterPreJoinFuture to acquire the same lock when removing
state so they cannot interleave and leave stale flags. Ensure methods
referenced: approvePreJoinForceLogin, unregisterPreJoinFuture, clear and the
collections pendingPreJoinByName, pendingPreJoinFutures, pendingForceLogins use
the same synchronization.
In `@authme-core/src/main/resources/messages/messages_de.yml`:
- Line 1: The German locale file contains mojibake (e.g., strings like
"bestätigen", "für", "Überprüfung") due to wrong encoding; reopen the
messages_de.yml, convert and save it as UTF-8 (without a mismatched BOM),
restore the original German umlauts and special characters in all message values
(registration, login, recovery, 2FA, premium messages) and re-run a quick grep
for the example corrupted tokens to confirm no stray mojibake remains.
---
Outside diff comments:
In `@authme-core/src/main/resources/messages/messages_br.yml`:
- Around line 1-241: This file contains double-encoded UTF-8 (mojibake) in many
translation values; revert the edited translations back to their original pre-PR
Portuguese strings for keys like registration.register_request,
password.match_error, login.login_request, error.unregistered_user,
on_join_validation.invalid_name_case, recovery.code.code_sent, dialog.* and
other top-level sections, then re-add only the premium block (premium.* keys)
using a UTF-8-aware editor and save the file as UTF-8 (no BOM) so accented
characters (e.g., Tradução, usuários, Você, não, reconecte-se) are stored
correctly; ensure no other keys are changed beyond restoring correct Portuguese
text.
In `@authme-core/src/main/resources/messages/messages_et.yml`:
- Around line 1-238: Many Estonian strings were double-encoded (mojibake) in
keys like registration.register_request, password.match_error,
dialog.button.cancel, time.day and others; revert all edits to the pre-existing
keys (everything except the premium block) so their original UTF-8 text is
restored, then re-add only the premium block (premium.feature_disabled ...
premium.pending_fail) ensuring the file is saved in UTF-8; open/save with a
UTF-8-aware editor or run a transliteration fix on the corrupted values (e.g.
for a string s: s.encode('latin-1').decode('utf-8')) to convert garbled entries
back to proper UTF-8 before committing.
---
Duplicate comments:
In `@authme-core/src/main/java/fr/xephi/authme/service/PremiumLoginVerifier.java`:
- Around line 89-103: The pending-entry TTL is only enforced when starting
verifications, so reads can return stale entries; update all read/remove methods
(e.g., hasPending, getPendingUsername, getPendingPlayerUUID and the methods
around lines 133-138 such as completeVerification/removePending) to check expiry
before returning data. Implement this by either invoking
evictStalePendingEntries() at the start of each read/remove method or by
checking the PendingVerification's timestamp against PENDING_TTL_MS (and
removing the map entry if expired) so expired handshakes are treated as absent
and not returned or accepted.
In `@authme-core/src/main/java/fr/xephi/authme/service/PremiumService.java`:
- Around line 158-160: Normalize the admin target once at the start of the async
task and use that normalized key for all persistence/cache calls: replace uses
of the raw playerName inside the bukkitService.runTaskOptionallyAsync lambda
(where dataSource.getAuth(playerName) and
playerCache.isAuthenticated(playerName) are called) with a single normalized
variable (e.g., normalizedPlayer or normalizedName) and use that same variable
for any subsequent lookups/updates; apply the same change to the other block
referenced (the logic around lines 228-249) so all dataSource and playerCache
interactions consistently use the normalized key.
In `@docs/premium.md`:
- Around line 54-79: Add a language tag to the fenced code block that begins
with "Client Server (AuthMe + PacketEvents)
Mojang" so the markdown linter (MD040) is satisfied; change the opening fence
from ``` to ```text (or another appropriate language such as ```nohighlight) and
leave the block contents unchanged.
---
Nitpick comments:
In `@authme-core/src/main/resources/plugin.yml`:
- Around line 326-338: The permission node authme.player.premium currently has
default: true (as does authme.player.freemium), which means any player can run
/premium to attempt premium enrollment; update the authme.player.premium
description (or README/wiki) to explicitly state that default:true allows all
players to self-enroll and that server operators must explicitly deny this
permission to prevent self-enrollment, and mention that enrollment is still
gated by Mojang session verification for cryptographic protection.
In
`@authme-core/src/test/java/fr/xephi/authme/process/join/AsynchronousJoinTest.java`:
- Around line 97-111: The test setup stubs PremiumSettings.ENABLE_PREMIUM to
false so AsynchronousJoin.canBypassWithPremium() is never exercised; add new
unit tests in AsynchronousJoinTest that set
given(service.getProperty(PremiumSettings.ENABLE_PREMI)).willReturn(true) and
exercise AsynchronousJoin.canBypassWithPremium() for (1) a v4 UUID path that
should bypass immediately (proxy-verified), (2) a v3 UUID path that invokes the
PremiumLoginVerifier mock (verify interactions with PremiumLoginVerifier) and
(3) pending verification flows where the verifier returns success and failure to
assert both acceptance and denial handling; update/instantiate the
PremiumLoginVerifier mock behavior in each test and reuse existing helper
methods from setUp() to schedule tasks as needed.
In
`@authme-core/src/test/java/fr/xephi/authme/service/bungeecord/BungeeReceiverTest.java`:
- Around line 99-127: Add tests that exercise the premium verification branch by
stubbing a pending premium UUID and the player's UniqueId: create three new test
cases (modeled on
shouldQueueSessionAndForceLoginWhenPerformLoginReceivedForOnlinePlayer) where
you set pendingPremiumCache to return a UUID for playerName, then set the mocked
Player.getUniqueId() to (a) a matching v4 UUID, (b) a v3/offline-mode UUID, and
(c) a non-matching UUID; in each test call receiver.onPluginMessageReceived(...)
and verify the expected interactions with
proxySessionManager.processProxySessionMessage, management.forceLoginFromProxy
and bungeeSender.sendAuthMeBungeecordMessage and also verify pendingPremiumCache
removal or premiumService invocation as appropriate to each branch so the
premium verification logic in BungeeReceiver is covered.
In `@docs/premium.md`:
- Around line 183-187: Clarify that premium bypass is checked by Mojang UUID
while AuthMe stores accounts by lowercase player name, so after a username
change the premium check will still recognize the player (UUID), but AuthMe will
not automatically map the new name to the old name-keyed account; instruct
readers to either run /authme premium to re-link the new name to their premium
status or use their server's AuthMe account-migration/rename feature (or a
plugin setting that enables UUID-based account linking) to migrate the old name
record to the new lowercase name so the player does not need to fully
re-register.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7c94c52d-d16e-411c-b630-0c3f91eb22a5
📒 Files selected for processing (107)
README.mdauthme-bungee/src/main/java/fr/xephi/authme/bungee/AuthMeBungeePlugin.javaauthme-bungee/src/main/java/fr/xephi/authme/bungee/BungeeProxyBridge.javaauthme-bungee/src/main/java/fr/xephi/authme/bungee/BungeeProxyConfiguration.javaauthme-bungee/src/main/java/fr/xephi/authme/bungee/config/BungeeConfigProperties.javaauthme-bungee/src/test/java/fr/xephi/authme/bungee/BungeeProxyBridgeTest.javaauthme-bungee/src/test/java/fr/xephi/authme/bungee/BungeeReloadCommandTest.javaauthme-core/src/main/java/fr/xephi/authme/command/CommandInitializer.javaauthme-core/src/main/java/fr/xephi/authme/command/executable/authme/ForceLoginCommand.javaauthme-core/src/main/java/fr/xephi/authme/command/executable/authme/SetFreemiumAdminCommand.javaauthme-core/src/main/java/fr/xephi/authme/command/executable/authme/SetPremiumAdminCommand.javaauthme-core/src/main/java/fr/xephi/authme/command/executable/premium/FreemiumCommand.javaauthme-core/src/main/java/fr/xephi/authme/command/executable/premium/PremiumCommand.javaauthme-core/src/main/java/fr/xephi/authme/data/auth/PlayerAuth.javaauthme-core/src/main/java/fr/xephi/authme/datasource/AbstractSqlDataSource.javaauthme-core/src/main/java/fr/xephi/authme/datasource/CacheDataSource.javaauthme-core/src/main/java/fr/xephi/authme/datasource/Columns.javaauthme-core/src/main/java/fr/xephi/authme/datasource/DataSource.javaauthme-core/src/main/java/fr/xephi/authme/datasource/MySQL.javaauthme-core/src/main/java/fr/xephi/authme/datasource/PostgreSqlDataSource.javaauthme-core/src/main/java/fr/xephi/authme/datasource/SQLite.javaauthme-core/src/main/java/fr/xephi/authme/datasource/columnshandler/AuthMeColumns.javaauthme-core/src/main/java/fr/xephi/authme/listener/packetevents/AesCfb8Decoder.javaauthme-core/src/main/java/fr/xephi/authme/listener/packetevents/AesCfb8Encoder.javaauthme-core/src/main/java/fr/xephi/authme/listener/packetevents/PacketEventsListenerRegistry.javaauthme-core/src/main/java/fr/xephi/authme/listener/packetevents/PacketEventsService.javaauthme-core/src/main/java/fr/xephi/authme/listener/packetevents/PremiumVerificationPacketListener.javaauthme-core/src/main/java/fr/xephi/authme/message/MessageKey.javaauthme-core/src/main/java/fr/xephi/authme/message/updater/MessageUpdater.javaauthme-core/src/main/java/fr/xephi/authme/permission/AdminPermission.javaauthme-core/src/main/java/fr/xephi/authme/permission/PlayerPermission.javaauthme-core/src/main/java/fr/xephi/authme/platform/AbstractSpigotPlatformAdapter.javaauthme-core/src/main/java/fr/xephi/authme/platform/PacketInterceptionAdapter.javaauthme-core/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.javaauthme-core/src/main/java/fr/xephi/authme/service/MojangApiService.javaauthme-core/src/main/java/fr/xephi/authme/service/PendingPremiumCache.javaauthme-core/src/main/java/fr/xephi/authme/service/PreJoinDialogService.javaauthme-core/src/main/java/fr/xephi/authme/service/PremiumLoginVerifier.javaauthme-core/src/main/java/fr/xephi/authme/service/PremiumService.javaauthme-core/src/main/java/fr/xephi/authme/service/bungeecord/BungeeReceiver.javaauthme-core/src/main/java/fr/xephi/authme/service/bungeecord/BungeeSender.javaauthme-core/src/main/java/fr/xephi/authme/service/bungeecord/MessageType.javaauthme-core/src/main/java/fr/xephi/authme/settings/properties/AuthMeSettingsRetriever.javaauthme-core/src/main/java/fr/xephi/authme/settings/properties/DatabaseSettings.javaauthme-core/src/main/java/fr/xephi/authme/settings/properties/PremiumSettings.javaauthme-core/src/main/resources/messages/messages_bg.ymlauthme-core/src/main/resources/messages/messages_br.ymlauthme-core/src/main/resources/messages/messages_cz.ymlauthme-core/src/main/resources/messages/messages_de.ymlauthme-core/src/main/resources/messages/messages_en.ymlauthme-core/src/main/resources/messages/messages_eo.ymlauthme-core/src/main/resources/messages/messages_es.ymlauthme-core/src/main/resources/messages/messages_et.ymlauthme-core/src/main/resources/messages/messages_eu.ymlauthme-core/src/main/resources/messages/messages_fi.ymlauthme-core/src/main/resources/messages/messages_fr.ymlauthme-core/src/main/resources/messages/messages_gl.ymlauthme-core/src/main/resources/messages/messages_hu.ymlauthme-core/src/main/resources/messages/messages_id.ymlauthme-core/src/main/resources/messages/messages_it.ymlauthme-core/src/main/resources/messages/messages_ja.ymlauthme-core/src/main/resources/messages/messages_ko.ymlauthme-core/src/main/resources/messages/messages_lt.ymlauthme-core/src/main/resources/messages/messages_nl.ymlauthme-core/src/main/resources/messages/messages_pl.ymlauthme-core/src/main/resources/messages/messages_pt.ymlauthme-core/src/main/resources/messages/messages_ro.ymlauthme-core/src/main/resources/messages/messages_ru.ymlauthme-core/src/main/resources/messages/messages_si.ymlauthme-core/src/main/resources/messages/messages_sk.ymlauthme-core/src/main/resources/messages/messages_sr.ymlauthme-core/src/main/resources/messages/messages_tr.ymlauthme-core/src/main/resources/messages/messages_uk.ymlauthme-core/src/main/resources/messages/messages_vn.ymlauthme-core/src/main/resources/messages/messages_zhcn.ymlauthme-core/src/main/resources/messages/messages_zhhk.ymlauthme-core/src/main/resources/messages/messages_zhmc.ymlauthme-core/src/main/resources/messages/messages_zhtw.ymlauthme-core/src/main/resources/plugin.ymlauthme-core/src/test/java/fr/xephi/authme/command/CommandInitializerTest.javaauthme-core/src/test/java/fr/xephi/authme/command/executable/authme/ForceLoginCommandTest.javaauthme-core/src/test/java/fr/xephi/authme/platform/AbstractSpigotPlatformAdapterTest.javaauthme-core/src/test/java/fr/xephi/authme/process/join/AsynchronousJoinTest.javaauthme-core/src/test/java/fr/xephi/authme/service/PreJoinDialogServiceTest.javaauthme-core/src/test/java/fr/xephi/authme/service/bungeecord/BungeeReceiverTest.javaauthme-core/src/test/resources/fr/xephi/authme/datasource/sql-initialize.sqlauthme-core/src/test/resources/plugin.ymlauthme-folia/src/main/resources/plugin.ymlauthme-paper-common/pom.xmlauthme-paper-common/src/main/java/fr/xephi/authme/listener/PaperDialogFlowListener.javaauthme-paper-common/src/main/java/fr/xephi/authme/platform/AbstractPaperPlatformAdapter.javaauthme-paper-common/src/test/java/fr/xephi/authme/listener/PaperDialogFlowListenerTest.javaauthme-paper/src/main/resources/plugin.ymlauthme-spigot-1.21/src/main/resources/plugin.ymlauthme-spigot-legacy/src/main/resources/plugin.ymlauthme-velocity/src/main/java/fr/xephi/authme/velocity/AuthMeVelocityPlugin.javaauthme-velocity/src/main/java/fr/xephi/authme/velocity/VelocityProxyBridge.javaauthme-velocity/src/main/java/fr/xephi/authme/velocity/VelocityProxyConfiguration.javaauthme-velocity/src/main/java/fr/xephi/authme/velocity/config/VelocityConfigProperties.javaauthme-velocity/src/test/java/fr/xephi/authme/velocity/VelocityProxyBridgeTest.javadocs/commands.mddocs/config.mddocs/permission_nodes.mddocs/premium.mddocs/proxies/bungee/config.ymldocs/proxies/velocity/config.ymldocs/translations.md
✅ Files skipped from review due to trivial changes (30)
- docs/proxies/velocity/config.yml
- authme-core/src/main/java/fr/xephi/authme/datasource/Columns.java
- docs/config.md
- authme-core/src/main/java/fr/xephi/authme/data/auth/PlayerAuth.java
- authme-core/src/main/resources/messages/messages_gl.yml
- authme-core/src/main/java/fr/xephi/authme/datasource/MySQL.java
- authme-core/src/main/resources/messages/messages_ru.yml
- authme-bungee/src/main/java/fr/xephi/authme/bungee/AuthMeBungeePlugin.java
- authme-core/src/main/resources/messages/messages_ja.yml
- authme-core/src/main/resources/messages/messages_zhhk.yml
- authme-core/src/main/resources/messages/messages_bg.yml
- authme-core/src/main/resources/messages/messages_zhcn.yml
- authme-core/src/main/resources/messages/messages_es.yml
- authme-core/src/main/resources/messages/messages_zhmc.yml
- authme-core/src/main/java/fr/xephi/authme/command/executable/premium/PremiumCommand.java
- docs/translations.md
- authme-bungee/src/test/java/fr/xephi/authme/bungee/BungeeReloadCommandTest.java
- authme-core/src/main/java/fr/xephi/authme/datasource/PostgreSqlDataSource.java
- authme-core/src/main/java/fr/xephi/authme/service/MojangApiService.java
- authme-folia/src/main/resources/plugin.yml
- authme-core/src/main/java/fr/xephi/authme/permission/AdminPermission.java
- authme-core/src/main/resources/messages/messages_tr.yml
- authme-core/src/main/resources/messages/messages_it.yml
- authme-core/src/main/resources/messages/messages_nl.yml
- authme-core/src/main/resources/messages/messages_ko.yml
- docs/commands.md
- authme-bungee/src/main/java/fr/xephi/authme/bungee/config/BungeeConfigProperties.java
- authme-core/src/main/resources/messages/messages_fi.yml
- authme-core/src/main/resources/messages/messages_cz.yml
- authme-core/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java
🚧 Files skipped from review as they are similar to previous changes (44)
- authme-velocity/src/main/java/fr/xephi/authme/velocity/config/VelocityConfigProperties.java
- authme-core/src/main/resources/messages/messages_eu.yml
- authme-core/src/main/resources/messages/messages_sk.yml
- authme-core/src/main/java/fr/xephi/authme/settings/properties/PremiumSettings.java
- authme-core/src/test/resources/plugin.yml
- authme-paper-common/pom.xml
- docs/proxies/bungee/config.yml
- authme-core/src/main/java/fr/xephi/authme/settings/properties/DatabaseSettings.java
- authme-core/src/test/java/fr/xephi/authme/platform/AbstractSpigotPlatformAdapterTest.java
- authme-core/src/main/resources/messages/messages_id.yml
- authme-core/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java
- authme-core/src/main/resources/messages/messages_fr.yml
- authme-core/src/main/java/fr/xephi/authme/command/executable/authme/SetFreemiumAdminCommand.java
- authme-velocity/src/test/java/fr/xephi/authme/velocity/VelocityProxyBridgeTest.java
- authme-core/src/main/resources/messages/messages_vn.yml
- authme-core/src/main/java/fr/xephi/authme/command/CommandInitializer.java
- authme-core/src/main/resources/messages/messages_pt.yml
- authme-core/src/main/java/fr/xephi/authme/datasource/columnshandler/AuthMeColumns.java
- authme-core/src/main/java/fr/xephi/authme/datasource/SQLite.java
- authme-core/src/main/resources/messages/messages_hu.yml
- authme-core/src/main/resources/messages/messages_en.yml
- authme-core/src/main/resources/messages/messages_zhtw.yml
- authme-core/src/main/java/fr/xephi/authme/settings/properties/AuthMeSettingsRetriever.java
- authme-core/src/main/java/fr/xephi/authme/listener/packetevents/PacketEventsListenerRegistry.java
- authme-spigot-legacy/src/main/resources/plugin.yml
- authme-core/src/main/resources/messages/messages_pl.yml
- authme-paper-common/src/main/java/fr/xephi/authme/listener/PaperDialogFlowListener.java
- authme-bungee/src/test/java/fr/xephi/authme/bungee/BungeeProxyBridgeTest.java
- README.md
- authme-core/src/main/java/fr/xephi/authme/service/bungeecord/MessageType.java
- authme-core/src/main/java/fr/xephi/authme/command/executable/premium/FreemiumCommand.java
- authme-bungee/src/main/java/fr/xephi/authme/bungee/BungeeProxyConfiguration.java
- authme-core/src/main/resources/messages/messages_lt.yml
- authme-paper-common/src/test/java/fr/xephi/authme/listener/PaperDialogFlowListenerTest.java
- authme-core/src/main/resources/messages/messages_uk.yml
- authme-core/src/main/java/fr/xephi/authme/listener/packetevents/PacketEventsService.java
- authme-core/src/main/java/fr/xephi/authme/message/updater/MessageUpdater.java
- authme-velocity/src/main/java/fr/xephi/authme/velocity/VelocityProxyBridge.java
- authme-paper/src/main/resources/plugin.yml
- authme-core/src/main/java/fr/xephi/authme/listener/packetevents/AesCfb8Encoder.java
- authme-paper-common/src/main/java/fr/xephi/authme/platform/AbstractPaperPlatformAdapter.java
- authme-velocity/src/main/java/fr/xephi/authme/velocity/AuthMeVelocityPlugin.java
- authme-velocity/src/main/java/fr/xephi/authme/velocity/VelocityProxyConfiguration.java
- authme-spigot-1.21/src/main/resources/plugin.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build_and_test (21)
- GitHub Check: build_and_test (21)
🧰 Additional context used
🪛 ast-grep (0.42.1)
authme-core/src/main/java/fr/xephi/authme/service/PremiumLoginVerifier.java
[warning] 193-193: Detected SHA1 hash algorithm which is considered insecure. SHA1 is not collision resistant and is therefore not suitable as a cryptographic signature. Instead, use PBKDF2 for password hashing or SHA256 or SHA512 for other hash function applications.
Context: MessageDigest.getInstance("SHA-1")
Note: [CWE-328] Use of Weak Hash. [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
(use-of-sha1-java)
[warning] 187-187: Cipher in ECB mode is detected. ECB mode produces the same output for the same input each time which allows an attacker to intercept and replay the data. Further, ECB mode does not provide any integrity checking. See https://find-sec-bugs.github.io/bugs.htm#CIPHER_INTEGRITY.
Context: Cipher cipher = Cipher.getInstance("RSA/ECB/PKCS1Padding");
Note: [CWE-327] Use of a Broken or Risky Cryptographic Algorithm. [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
(ecb-cipher-java)
🪛 LanguageTool
docs/permission_nodes.md
[style] ~31-~31: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...mand to set the first AuthMe spawn. - authme.admin.setfreemium – Administrator com...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~32-~32: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... disable premium mode for a player. - authme.admin.setpremium – Administrator comm...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~67-~67: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ssion to see the own email address. - authme.player.freemium – Permission to disab...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~68-~68: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...Permission to disable premium mode. - authme.player.login – Command permission to ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~69-~69: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...in** – Command permission to login. - authme.player.logout – Command permission to...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~69-~69: Ensure spelling is correct
Context: ...player.logout** – Command permission to logout. - authme.player.premium – Permissi...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~70-~70: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...t** – Command permission to logout. - authme.player.premium – Permission to enable...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.22.1)
docs/premium.md
[warning] 54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (20)
authme-core/src/main/resources/messages/messages_si.yml (1)
217-238: LGTM — all premium message keys accounted for with correct placeholders.All 18
PREMIUM_*/PREMIUM_ADMIN_*constants declared inMessageKey.java(lines 456–507) map exactly to the keys introduced here. Every admin-scoped key that carries"%name"in itsMessageKeydeclaration includes the placeholder, andpremium.admin.kick_reason(no declared placeholder) correctly omits it. Thepending_kick/pending_failkeys are correctly nested directly underpremium:rather than underpremium.admin:, matching their paths in the enum.authme-core/src/main/java/fr/xephi/authme/permission/PlayerPermission.java (1)
86-96: LGTM!Enum syntax is correct — the
;terminator was properly moved fromDISABLE_TWO_FACTOR_AUTHto the new last constantUSE_FREEMIUM. Node strings are consistent with their declarations inplugin.ymlanddocs/permission_nodes.md.authme-core/src/main/resources/plugin.yml (3)
42-42: LGTM!The
/authmeusage string correctly appendspremium|freemium.
82-87: LGTM!
/premiumand/freemiumare correctly registered as top-level commands, consistent with the plugin's existing command registration pattern.
112-113: LGTM!
authme.admin.setfreemiumandauthme.admin.setpremiumare correctly inserted alphabetically in theauthme.admin.*children block and theirdefault: opdefinitions are consistent with existing admin permission nodes and the test fixture.Also applies to: 181-186
docs/permission_nodes.md (1)
2-2: LGTM!New entries (
authme.admin.setfreemium,authme.admin.setpremium,authme.player.freemium,authme.player.premium) are correctly inserted in alphabetical order and their descriptions matchplugin.ymlverbatim. The LanguageTool style/grammar hints are false positives for this auto-generated list format.Also applies to: 31-32, 67-83
authme-core/src/test/java/fr/xephi/authme/service/PreJoinDialogServiceTest.java (1)
17-85: LGTM!All six test methods cover the key behavioral contracts of
PreJoinDialogService: single-consume semantics, future completion on approval, correct false returns for missing/unregistered state, and full-state clearing. The test assertions are correct, including thefuture.getNow("sentinel")check at Line 39 (sinceisDone()is already asserted true,getNowreturns the actual completed valuenullrather than the sentinel).authme-core/src/main/resources/messages/messages_ro.yml (1)
217-237: LGTM — past review issue resolved, premium section complete.All 18
premium.*andpremium.admin.*keys are present and correctly structured under the new top-levelpremiumblock. The previously reported nesting bug (admin.spawn.first_not_definedaccidentally placed underpremium.admin) no longer exists: Line 215 correctly placesfirst_not_definedunderadmin.spawn.authme-core/src/main/java/fr/xephi/authme/message/MessageKey.java (1)
455-507: LGTM — all 18 new premium constants are correct.Key/tag mappings align exactly with their callsites in
PremiumService(verified against all four usage snippets): keys with%nameinterpolation declare the"%name"tag, keys used only withretrieveSingleorsendwith no replacements declare no tags, and all YAML key paths match the newpremium.*blocks added across the locale files.authme-core/src/main/resources/messages/messages_sr.yml (1)
218-238: LGTM — Serbian premium section is complete and correctly encoded.All 18
premium.*andpremium.admin.*keys are present with correct Cyrillic text (no encoding corruption). The 2-space indent is inconsistent with the 4-space style used in the rest of this file, but YAML parsers (including snakeyaml) handle mixed indent levels without issues.authme-core/src/main/resources/messages/messages_eo.yml (1)
218-238: LGTM — Esperanto premium section is complete and correctly encoded.All 18 premium keys are present, Esperanto-specific diacritics (
ĝ,ĉ,ŝ,ĵ,ĉ) are correctly encoded, and indentation is consistent with the rest of the file.authme-core/src/main/java/fr/xephi/authme/service/bungeecord/BungeeReceiver.java (1)
37-60: LGTM!The new dependency injections for
DataSource,PendingPremiumCache, andPremiumServiceare properly added and stored, enabling the premium verification flow.authme-core/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java (6)
118-126: LGTM!The new dependency injections for premium verification (
PremiumLoginVerifier,PendingPremiumCache,PremiumService) are properly declared and will be used in thecanBypassWithPremiumflow.
189-210: LGTM!The use of
forceLoginFromProxy()for proxy-session resumption (line 197) and premium bypass with proxy enabled (line 205) is appropriate. The quiet mode prevents "already logged in" errors whenBungeeReceiver.performLogin()has already completed the login concurrently.
227-232: LGTM!This guard prevents the race condition where
BungeeReceiver.performLogin()could authenticate the player before the async join task reaches limbo creation. Without this, the player would be frozen in limbo permanently.
259-263: LGTM!A second authentication guard inside the sync task provides defense-in-depth against the timing window between scheduling and execution.
284-287: LGTM!The
pendingForceLoginpath correctly triggers an asyncforceLogin()call after limbo creation, enabling pre-join dialog approval to complete the login flow.
350-398: LGTM!The
canBypassWithPremiummethod implements a sound security model:
- UUID v4 (online-mode) is trusted when the backend is proxy-firewalled
- UUID v3 (offline-mode) requires cryptographic verification via
PremiumLoginVerifier- Pending premium verification correctly compares the connecting player's UUID against the stored pending UUID before finalizing
The
bungeeSender.sendPremiumUnsetcall at line 383 is safe even without a proxy, asBungeeSenderguards against disabled state internally.authme-core/src/test/java/fr/xephi/authme/process/join/AsynchronousJoinTest.java (2)
197-214: LGTM!This test correctly verifies that proxy session resumption uses
forceLoginFromProxy()(quiet mode) and skips limbo creation, preventing conflicts with concurrentBungeeReceiverlogins.
236-253: LGTM!This test correctly verifies the
pendingForceLoginpath: limbo creation followed byforceLogin(), without triggering password-based login or dialogs.
| if (player == null || !player.isOnline()) { | ||
| messages.send(sender, MessageKey.FORCE_LOGIN_PLAYER_OFFLINE); | ||
| // Player may be blocked in the pre-join dialog (Paper/Folia configuration phase). | ||
| // Approving the force-login completes the blocking future so the player proceeds to | ||
| // PLAY state, where AsynchronousJoin will call forceLogin() on their behalf. | ||
| if (preJoinDialogService.approvePreJoinForceLogin(playerName.toLowerCase(Locale.ROOT))) { | ||
| messages.send(sender, MessageKey.FORCE_LOGIN_SUCCESS, playerName); | ||
| } else { | ||
| messages.send(sender, MessageKey.FORCE_LOGIN_PLAYER_OFFLINE); | ||
| } |
There was a problem hiding this comment.
Don't bypass CAN_LOGIN_BE_FORCED in the pre-join path.
This branch force-approves pre-join players by name before the target's CAN_LOGIN_BE_FORCED protection is checked, so accounts that deny forced login lose that protection during the configuration phase. Please add an equivalent permission check for pending pre-join connections, or fail closed here until one exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@authme-core/src/main/java/fr/xephi/authme/command/executable/authme/ForceLoginCommand.java`
around lines 44 - 52, The pre-join branch calls
preJoinDialogService.approvePreJoinForceLogin(...) without checking the target's
CAN_LOGIN_BE_FORCED permission, so add a permission check before approving:
query the permission/authorization routine used elsewhere (e.g.,
PermissionNode.CAN_LOGIN_BE_FORCED or the same permission-check helper used for
online players) for playerName.toLowerCase(Locale.ROOT) and only call
preJoinDialogService.approvePreJoinForceLogin(...) if that check allows forced
logins; otherwise treat as denied and send the existing failure message
(MessageKey.FORCE_LOGIN_PLAYER_OFFLINE or a suitable denial message). Ensure you
reference the same permission constant/method as used for the online-player path
and keep the lowercasing key consistent.
| // DB lookup must happen off the Netty event loop to avoid blocking | ||
| CompletableFuture.runAsync(() -> { | ||
| String lowerName = username.toLowerCase(Locale.ROOT); | ||
| fr.xephi.authme.data.auth.PlayerAuth auth = dataSource.getAuth(lowerName); | ||
|
|
||
| boolean isPremium = auth != null && auth.isPremium(); | ||
| boolean isPending = !isPremium && pendingPremiumCache.isPending(username); | ||
|
|
||
| if (isPremium || isPending) { | ||
| byte[] verifyToken = loginVerifier.startVerification(connectionKey, username, playerUUID); | ||
| WrapperLoginServerEncryptionRequest encReq = new WrapperLoginServerEncryptionRequest( | ||
| "", loginVerifier.getPublicKey(), verifyToken, true); | ||
| user.sendPacket(encReq); | ||
| } else { | ||
| // Not premium (and not pending) — resume normal login without verification | ||
| resumeLogin(user, username, clientVersion, playerUUID); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n authme-core/src/main/java/fr/xephi/authme/listener/packetevents/PremiumVerificationPacketListener.javaRepository: TuxCoding/AuthMeReloaded
Length of output: 12583
Always resolve the cancelled login on async failures.
LOGIN_START is cancelled before this task runs, but exceptions inside runAsync(...) are never observed. A DB/cache failure here leaves the connection stuck because neither resumeLogin(...) nor a disconnect path runs. Wrap the task body and explicitly resume or fail the login on error (follow the pattern used in handleEncryptionResponse() at lines 158–168).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@authme-core/src/main/java/fr/xephi/authme/listener/packetevents/PremiumVerificationPacketListener.java`
around lines 88 - 105, The async task started in
PremiumVerificationPacketListener must handle exceptions so cancelled
LOGIN_START doesn't leave connections hanging; wrap the
CompletableFuture.runAsync body in a try/catch, and on any thrown exception call
the same recovery path used in handleEncryptionResponse() — either
resumeLogin(user, username, clientVersion, playerUUID) or explicitly
fail/disconnect the user (using the existing disconnect/resume helpers) and log
the error; reference the pendingPremiumCache check, dataSource.getAuth(...),
loginVerifier.startVerification(...), and resumeLogin(...) when implementing the
catch to ensure every error path resumes or terminates the login flow.
| UUID pendingUuid = pendingPremiumCache.removePending(name); | ||
| if (pendingUuid != null) { | ||
| UUID playerId = player.getUniqueId(); | ||
| if (playerId.version() == 4) { | ||
| premiumService.finalizePendingPremium(player, playerId); | ||
| } else { | ||
| // Unexpected: proxy sent PERFORM_LOGIN but player has an offline UUID. | ||
| // Discard the pending state and let the player use a password. | ||
| logger.warning("Received PERFORM_LOGIN for pending-premium player " + name | ||
| + " but their UUID is not v4 — discarding pending verification"); | ||
| bungeeSender.sendPremiumUnset(name); | ||
| } |
There was a problem hiding this comment.
Missing UUID comparison before finalizing premium verification.
At line 168, finalizePendingPremium is called with playerId without verifying it matches pendingUuid. This differs from AsynchronousJoin.canBypassWithPremium() (line 370) which correctly compares playerId.equals(pendingUuid) before proceeding.
Without this check, a player could theoretically have a pending premium entry for one Mojang UUID but connect with a different Mojang account, and the wrong UUID would be stored.
Proposed fix
UUID pendingUuid = pendingPremiumCache.removePending(name);
if (pendingUuid != null) {
UUID playerId = player.getUniqueId();
- if (playerId.version() == 4) {
+ if (playerId.version() == 4 && playerId.equals(pendingUuid)) {
premiumService.finalizePendingPremium(player, playerId);
- } else {
+ } else if (playerId.version() != 4) {
// Unexpected: proxy sent PERFORM_LOGIN but player has an offline UUID.
// Discard the pending state and let the player use a password.
logger.warning("Received PERFORM_LOGIN for pending-premium player " + name
+ " but their UUID is not v4 — discarding pending verification");
bungeeSender.sendPremiumUnset(name);
+ } else {
+ // UUID mismatch: player connected with a different Mojang account
+ logger.warning("Received PERFORM_LOGIN for pending-premium player " + name
+ + " but their UUID does not match the pending UUID — discarding pending verification");
+ bungeeSender.sendPremiumUnset(name);
}
}📝 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.
| UUID pendingUuid = pendingPremiumCache.removePending(name); | |
| if (pendingUuid != null) { | |
| UUID playerId = player.getUniqueId(); | |
| if (playerId.version() == 4) { | |
| premiumService.finalizePendingPremium(player, playerId); | |
| } else { | |
| // Unexpected: proxy sent PERFORM_LOGIN but player has an offline UUID. | |
| // Discard the pending state and let the player use a password. | |
| logger.warning("Received PERFORM_LOGIN for pending-premium player " + name | |
| + " but their UUID is not v4 — discarding pending verification"); | |
| bungeeSender.sendPremiumUnset(name); | |
| } | |
| UUID pendingUuid = pendingPremiumCache.removePending(name); | |
| if (pendingUuid != null) { | |
| UUID playerId = player.getUniqueId(); | |
| if (playerId.version() == 4 && playerId.equals(pendingUuid)) { | |
| premiumService.finalizePendingPremium(player, playerId); | |
| } else if (playerId.version() != 4) { | |
| // Unexpected: proxy sent PERFORM_LOGIN but player has an offline UUID. | |
| // Discard the pending state and let the player use a password. | |
| logger.warning("Received PERFORM_LOGIN for pending-premium player " + name | |
| " but their UUID is not v4 — discarding pending verification"); | |
| bungeeSender.sendPremiumUnset(name); | |
| } else { | |
| // UUID mismatch: player connected with a different Mojang account | |
| logger.warning("Received PERFORM_LOGIN for pending-premium player " + name | |
| " but their UUID does not match the pending UUID — discarding pending verification"); | |
| bungeeSender.sendPremiumUnset(name); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@authme-core/src/main/java/fr/xephi/authme/service/bungeecord/BungeeReceiver.java`
around lines 164 - 175, In BungeeReceiver, before calling
premiumService.finalizePendingPremium(player, playerId) (after
pendingPremiumCache.removePending(name)), verify that the player UUID returned
by player.getUniqueId() equals the pendingUuid removed from the cache; if they
match call finalizePendingPremium, otherwise log a warning similar to the
existing message and call bungeeSender.sendPremiumUnset(name) to discard the
pending state — i.e., replace the unconditional finalizePendingPremium call with
an equality check using playerId.equals(pendingUuid) and the same fallback
handling used for non-v4 UUIDs.
2 participants
Summary by CodeRabbit
New Features
Configuration
Localization
Documentation