Ichbinjoe / MCAuthenticator

2FA for Minecraft!
https://www.spigotmc.org/resources/mcauthenticator.18727/
GNU General Public License v3.0
42 stars 15 forks source link

Fix players not being able to authenticate if using BungeeCord and VentureChat #15

Closed leMaik closed 8 years ago

leMaik commented 8 years ago

This probably fixes issues with other BungeeCord-based global chat plugins as well.

Ichbinjoe commented 8 years ago

How does this fix anything? Could you explain your rationale?

Or actually, explain in depth what the issue is and how this commit fixes it.

leMaik commented 8 years ago

My issue is that players that have 2FA enabled can't use the chat or any commands when joining the server and also, they can't authenticate. I use VentureChat. This only occurs if I use this with bungee mode.

The lines I removed seem to fix this by allowing players to authenticate locally on the server even if bungee is enabled. The authentication is then propagated to BungeeCord (if I understood your code correctly) and voilà: Authentication works.

This fix may not be perfect, but it seems to resolve the issue. :smile: It would be great if you had a better solution, though!

Edit: Before, all AsyncChatEvents were just canceled if bungee mode was enabled and the player wasn't authenticated... That doesn't seem right to me.

Ichbinjoe commented 8 years ago

So cancelling chat before authentication is by design - if a compromised staff member account comes on the server, you would not want them chatting with players, causing PR issues or what not. Possibly an issue I will open is configurable checks when not authenticated - we seem to at the moment be taking a 'broad sweep' of all checks we can conceive.

However, this is not the primary issue. The way bungee mode works is that when a chat message is received, it is forwarded over a plugin message channel to the backend bukkit server where the message is authenticated. All chat/commands are 'wrapped' through this medium to the backend server - thus, when the network is in 'bungee' mode, there would be no conceivable use case for when a player should be able to chat while not authenticated, because they wouldn't be chatting - their messages are getting forwarded over. This is just protection actually from backend login attacks - and hence why I do not want to modify the code in this way.

Possibly it is how VentureChat works - VentureChat may be intercepting the chat message before MCAuthenticator is, allowing users to chat but not authenticate. If this is the case, lowing the chat listener priority bungee side would probably fix this.

I'm trying to find the best solution while not harming the integrity of the security put in place.

leMaik commented 8 years ago

I just created #16, sorry for opening this PR that doesn't really fix this issue. Oh, and thanks a lot for your explanation! :+1: