games647 / FastLogin

Checks if a minecraft player has a valid paid account. If so, they can skip offline authentication automatically. (premium auto login)
https://www.spigotmc.org/resources/fastlogin.14153
MIT License
498 stars 121 forks source link

[API]"autoRegister" not working #1197

Closed ShadowOfHeaven-Me closed 4 months ago

ShadowOfHeaven-Me commented 5 months ago

What happened?

With autoRegister: true the AuthPlugin#forceRegister was not invoked, and BukkitFastLoginPreLoginEvent#getProfile#isPremium returned false.

What did you expect?

BukkitFastLoginPreLoginEvent#getProfile#isPremium should return true, as that allows the plugin to determine whether it should even attempt to set-up the login authentication.

Steps to reproduce

Use autoRegister: true and debug the things I mentioned. The build is 1302, so the currently newest.

Plugin list

ViaVersion, ViaBackwards, ProtocolLib, ViaRewind, FastLogin, AlixSystem, PlugManX (PlugMan)

Configuration file

https://paste.md-5.net/neyabepiwa.yaml

Server log

[12:36:59] [Protocol Worker #1 - FastLogin - [recv: ENCRYPTION_BEGIN[class=PacketLoginInEncryptionBegin, id=1], START[class=PacketLoginInStart, id=0], send: ]/INFO]: [FastLogin] New packet START[class=PacketLoginInStart, id=0] from com.comphenix.protocol.injector.temporary.TemporaryPlayerInvocationHandler@55c3a721 [12:36:59] [Craft Scheduler Thread - 10/INFO]: [FastLogin] Handling player ShadowOfHeaven

Plugin version

1.12-SNAPSHOT-3925b66

Platform

Spigot

Relevance

ShadowOfHeaven-Me commented 5 months ago

I made sure that actually no methods in AuthPlugin are invoked

games647 commented 5 months ago

IsPremium is only set if the user is verified to be owner of the account. That is the connection is verified and the auth plugin successfully updated the user state.

ShadowOfHeaven-Me commented 5 months ago

FastLogin didn't even try to confirm that - isRegistered was not invoked

ShadowOfHeaven-Me commented 5 months ago

By my reasoning, if the AuthPlugin#isRegistered(String) returns false, meaning that the account is not registered, FastLogin should at the very least invoke AuthPlugin#forceRegister is autoRegister is set to true

games647 commented 5 months ago

By my reasoning, if the AuthPlugin#isRegistered(String) returns false, meaning that the account is not registered, FastLogin should at the very least invoke AuthPlugin#forceRegister is autoRegister is set to true

Yes it should does that, but only if isn't in its own database. The own storage takes priority. However, I just checked the implementation. The execution order is the following:

Considering a clean database:

  1. BukkitFastLoginPreLoginEvent fired
  2. isRegistered check
  3. enable online mode
  4. successful? -> forceRegister
  5. successful? -> setPremium(true)
ShadowOfHeaven-Me commented 5 months ago

So is the result of the player's premium status checked once, cached, and then reused?

games647 commented 5 months ago

That depends on your definition. Assuming auto-register is enabled, then we need to decided whether to enable online mode based on the username and connection details. If we request online mode from an offline player, the client will disconnect itself and we cannot change the disconnect message. Therefore, we need make an assumption if the username is registered at Mojang. This check is only performed once. For every new player connecting to the server. Everything after that uses the preference configuration from FastLogin's database.

If we would always make this check, it would easily hit the API rate-limit from Mojang, but also has an impact on players. If a player joins the server with an unregistered username, they will be registered as offline. If we would also make this check, and someone else registers an account with that username, the existing player client would disconnect on requesting the online mode.

Although it's possible to check/assume whether the first connection attempt failed (i.e. second-attempt-cracked), but it could still be a bad user experience if they are getting disconnects with something like This server is in onlinemode.

Nevertheless, the verification of the premium status for those online mode connections always happens.

ShadowOfHeaven-Me commented 5 months ago

I was asking because BukkitFastLoginPreLoginEvent#getProfile#isPremium returned false on a premium account. I was wondering if you could reimplement the following mechanic: The player's premium status is updated on each join. If the player is premium and the previous invocation of forceLogin/forceRegister occurred with the player having a confirmed premium account, then invoke forceLogin.

ShadowOfHeaven-Me commented 5 months ago

image I was also wondering if you could further explain this mechanic, since I haven't found the "allow-list" anywhere

games647 commented 5 months ago

You can consider StoredProfile.isPremium as a preference to use online mode connections.

I was asking because BukkitFastLoginPreLoginEvent#getProfile#isPremium returned false on a premium account

If the player registers a Mojang account after already playing on the server, the plugin won't upgrade the account automatically. This is implemented this way, because someone else could have registered a Mojang account. If we request a premium connection for the existing not premium player, their client will automatically disconnect with You are not authenticated with Mojang.net. So they will be locked ot of their account, because someone else registered a Mojang account.

The player's premium status is updated on each join.

It's important to differentiate between premium name checks and online mode verification. Although it's possible to always check whether the username is registered at Mojang (name check), we don't know if the connecting player is actual owner of that account. For that we need to perform an actual onlinemode authentication just like a vanilla server would do.

The player's premium status is updated on each join. If the player is premium and the previous invocation of forceLogin/forceRegister occurred with the player having a confirmed premium account, then invoke forceLogin.

Part of this already happening. Only if the premium verification and the auth plugin (forceLogin/forceRegister) reported successful operation, the premium value is set true meaning the onlinemode preference for future connections including forceLogin. Only that we cannot update the premium status on each connect without risiking that existing player client will disconnect if someone else registered Mojang account with their username.

image I was also wondering if you could further explain this mechanic, since I haven't found the "allow-list" anywhere

You could explicitly add entries to FastLogin database with /cracked. This is considered as an allow-list of all existing cracked player names.

ShadowOfHeaven-Me commented 5 months ago

Oh I see, that actually makes a lot of sense. Could you just fill me out on what connection details you use to base your assumption that the player is indeed a premium player, and thus you request the authentication from him? Another thing: Add PacketEvents support as a substitute for ProtocolLib and ProtocolSupport. I can post that as an additional enchancement issue

games647 commented 5 months ago

Could you just fill me out on what connection details you use to base your assumption that the player is indeed a premium player, and thus you request the authentication from him?

It is actual only the player name (auto-register) and whether they already tried to join the server minutes earlier, but failed (second-attempt).